zibi2 wrote:
Thanks for the review, Uli.
**On the `CC_XPLINK_Int128` / `CCPassIndirect` removal:**
You are correct — the `CC_XPLINK_Int128` mutation to `v16i8` and the
`CCPassIndirect` path were unnecessary. We have simplified this in the upstream
patch to do exactly what you suggested: add `i128` directly to the `CCIfType`
lists alongside vectors. The updated patch removes `CC_XPLINK_Int128`,
`CCPassIndirect<i64>`, and `CC_SystemZ_I128Indirect` entirely.
One implementation detail worth noting: the `lowerI128ToGR128` call in
`LowerCall_XPLINK` needed a guard to distinguish between i128 assigned to a
GR128 pair (e.g. R2Q for vararg) versus i128 assigned to a VR128 vector
register (V24–V31). Without the guard, the decomposition into hi/lo GPR halves
fired incorrectly for the vector register case, generating a spurious
`vlgvg`/`vlvgp` round-trip. The fix is:
```cpp
if (VA.getLocVT() == MVT::i128 &&
!SystemZ::VR128BitRegClass.contains(VA.getLocReg()))
ArgValue = lowerI128ToGR128(DAG, ArgValue);
```
This is needed to correctly handle the i128 vararg case where
`CC_XPLINK64_Allocate128BitVararg` assigns to R2Q.
**On gating `i128` behind the vector facility:**
On z/OS, unlike Linux ELF, there is no vector emulation for pre-z13 targets —
vector types are not legal types when hasVector() is false. Since i128 uses
VR128BitRegClass (same as vectors), it similarly requires the vector facility.
**On the `err_drv_incompatible_arch` diagnostic:**
You're right. The diagnostic was based on a false premise: z/OS has no vector
emulation, but that doesn't matter when -mvx is present — +vector is applied on
top of the CPU-implied bits, so -march=arch10 -mvx produces a valid arch10 +
vector facility subtarget regardless. I'll remove the diagnostic in a follow-up
commit.
**One ABI note for awareness:**
You're right that this is a concern since i128 shipped in the 2.2 compiler.
Here's the concrete impact.
The simplification changes how GPRs are shadowed for i128 fixed arguments: the
old CC_XPLINK_Int128 did not shadow any GPRs, while the new code adds i128 to
CC_XPLINK_Shadow_Reg (consistent with 128-bit vectors, which shadow 2 GPRs).
This shifts the GPR slot seen by subsequent vararg arguments.
Concrete example — callee(i128 %a, double %d, ...) on z10:
Before — i128 via CC_XPLINK_Int128, no GPR shadow, double vararg lands in GPR2:
lg 0, 8(1)
lg 3, 0(1)
stg 0, 2264(4) ; store i128 high to stack temp
stg 3, 2256(4) ; store i128 low to stack temp
la 1, 2256(4) ; GPR1 = pointer to i128
lgdr 2, f0 ; double vararg -> GPR2
After — i128 in V24, shadows GPR1+GPR2, double vararg lands in GPR3:
lgdr 3, f0 ; double vararg -> GPR3
The callee sees double in GPR2 (old) vs GPR3 (new). Any caller compiled with
the 2.2 compiler that passes i128 as a fixed argument followed by f64 or vector
varargs will be mismatched against a callee compiled with the new code.
https://github.com/llvm/llvm-project/pull/223026
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
