Skip to content

Commit 2f51c0f

Browse files
JonPsson1Honey Goyal
authored andcommitted
[SystemZ] Handle IR struct arguments correctly. (llvm#169583)
- The size of the stack slot was previously computed in LowerCall() by using the original type, but that didn't work for a struct. Compute the size by looking at the VT of each part and the number of them instead. - All the members of a struct have the same OrigArgIndex, so it doesn't work to assume that following parts belong to a split argument until another OrigArgIndex is encountered. Use the isSplit() and isSplitEnd() flags instead. - Detect any scalar integer argumet >64 bits in CanLowerReturn() instead of just i128, in order to let all of them be passed on stack. Fixes llvm#168460
1 parent b43e7ba commit 2f51c0f

File tree

2 files changed

+1058
-26
lines changed

2 files changed

+1058
-26
lines changed

llvm/lib/Target/SystemZ/SystemZISelLowering.cpp

Lines changed: 54 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -1970,6 +1970,28 @@ SDValue SystemZTargetLowering::joinRegisterPartsIntoValue(
19701970
return SDValue();
19711971
}
19721972

1973+
// The first part of a split stack argument is at index I in Args (and
1974+
// ArgLocs). Return the type of a part and the number of them by reference.
1975+
template <class ArgTy>
1976+
static bool analyzeArgSplit(const SmallVectorImpl<ArgTy> &Args,
1977+
SmallVector<CCValAssign, 16> &ArgLocs, unsigned I,
1978+
MVT &PartVT, unsigned &NumParts) {
1979+
if (!Args[I].Flags.isSplit())
1980+
return false;
1981+
assert(I < ArgLocs.size() && ArgLocs.size() == Args.size() &&
1982+
"ArgLocs havoc.");
1983+
PartVT = ArgLocs[I].getValVT();
1984+
NumParts = 1;
1985+
for (unsigned PartIdx = I + 1;; ++PartIdx) {
1986+
assert(PartIdx != ArgLocs.size() && "SplitEnd not found.");
1987+
assert(ArgLocs[PartIdx].getValVT() == PartVT && "Unsupported split.");
1988+
++NumParts;
1989+
if (Args[PartIdx].Flags.isSplitEnd())
1990+
break;
1991+
}
1992+
return true;
1993+
}
1994+
19731995
SDValue SystemZTargetLowering::LowerFormalArguments(
19741996
SDValue Chain, CallingConv::ID CallConv, bool IsVarArg,
19751997
const SmallVectorImpl<ISD::InputArg> &Ins, const SDLoc &DL,
@@ -2074,16 +2096,26 @@ SDValue SystemZTargetLowering::LowerFormalArguments(
20742096
MachinePointerInfo()));
20752097
// If the original argument was split (e.g. i128), we need
20762098
// to load all parts of it here (using the same address).
2077-
unsigned ArgIndex = Ins[I].OrigArgIndex;
2078-
assert (Ins[I].PartOffset == 0);
2079-
while (I + 1 != E && Ins[I + 1].OrigArgIndex == ArgIndex) {
2080-
CCValAssign &PartVA = ArgLocs[I + 1];
2081-
unsigned PartOffset = Ins[I + 1].PartOffset;
2082-
SDValue Address = DAG.getNode(ISD::ADD, DL, PtrVT, ArgValue,
2083-
DAG.getIntPtrConstant(PartOffset, DL));
2084-
InVals.push_back(DAG.getLoad(PartVA.getValVT(), DL, Chain, Address,
2085-
MachinePointerInfo()));
2086-
++I;
2099+
MVT PartVT;
2100+
unsigned NumParts;
2101+
if (analyzeArgSplit(Ins, ArgLocs, I, PartVT, NumParts)) {
2102+
// TODO: It is strange that while LowerCallTo() sets the PartOffset
2103+
// relative to the first split part LowerArguments() sets the offset
2104+
// from the beginning of the struct. So with {i32, i256}, the
2105+
// PartOffset for the i256 parts are differently handled. Try to
2106+
// remove that difference and use PartOffset directly here (instead
2107+
// of SplitBaseOffs).
2108+
unsigned SplitBaseOffs = Ins[I].PartOffset;
2109+
for (unsigned PartIdx = 1; PartIdx < NumParts; ++PartIdx) {
2110+
++I;
2111+
CCValAssign &PartVA = ArgLocs[I];
2112+
unsigned PartOffset = Ins[I].PartOffset - SplitBaseOffs;
2113+
SDValue Address = DAG.getNode(ISD::ADD, DL, PtrVT, ArgValue,
2114+
DAG.getIntPtrConstant(PartOffset, DL));
2115+
InVals.push_back(DAG.getLoad(PartVA.getValVT(), DL, Chain, Address,
2116+
MachinePointerInfo()));
2117+
assert(PartOffset && "Offset should be non-zero.");
2118+
}
20872119
}
20882120
} else
20892121
InVals.push_back(convertLocVTToValVT(DAG, DL, VA, Chain, ArgValue));
@@ -2319,37 +2351,33 @@ SystemZTargetLowering::LowerCall(CallLoweringInfo &CLI,
23192351

23202352
if (VA.getLocInfo() == CCValAssign::Indirect) {
23212353
// Store the argument in a stack slot and pass its address.
2322-
unsigned ArgIndex = Outs[I].OrigArgIndex;
23232354
EVT SlotVT;
2324-
if (I + 1 != E && Outs[I + 1].OrigArgIndex == ArgIndex) {
2325-
// Allocate the full stack space for a promoted (and split) argument.
2326-
Type *OrigArgType = CLI.Args[Outs[I].OrigArgIndex].Ty;
2327-
EVT OrigArgVT = getValueType(MF.getDataLayout(), OrigArgType);
2328-
MVT PartVT = getRegisterTypeForCallingConv(Ctx, CLI.CallConv, OrigArgVT);
2329-
unsigned N = getNumRegistersForCallingConv(Ctx, CLI.CallConv, OrigArgVT);
2330-
SlotVT = EVT::getIntegerVT(Ctx, PartVT.getSizeInBits() * N);
2331-
} else {
2355+
MVT PartVT;
2356+
unsigned NumParts = 1;
2357+
if (analyzeArgSplit(Outs, ArgLocs, I, PartVT, NumParts))
2358+
SlotVT = EVT::getIntegerVT(Ctx, PartVT.getSizeInBits() * NumParts);
2359+
else
23322360
SlotVT = Outs[I].VT;
2333-
}
23342361
SDValue SpillSlot = DAG.CreateStackTemporary(SlotVT);
23352362
int FI = cast<FrameIndexSDNode>(SpillSlot)->getIndex();
23362363
MemOpChains.push_back(
23372364
DAG.getStore(Chain, DL, ArgValue, SpillSlot,
23382365
MachinePointerInfo::getFixedStack(MF, FI)));
23392366
// If the original argument was split (e.g. i128), we need
23402367
// to store all parts of it here (and pass just one address).
2341-
assert (Outs[I].PartOffset == 0);
2342-
while (I + 1 != E && Outs[I + 1].OrigArgIndex == ArgIndex) {
2343-
SDValue PartValue = OutVals[I + 1];
2344-
unsigned PartOffset = Outs[I + 1].PartOffset;
2368+
assert(Outs[I].PartOffset == 0);
2369+
for (unsigned PartIdx = 1; PartIdx < NumParts; ++PartIdx) {
2370+
++I;
2371+
SDValue PartValue = OutVals[I];
2372+
unsigned PartOffset = Outs[I].PartOffset;
23452373
SDValue Address = DAG.getNode(ISD::ADD, DL, PtrVT, SpillSlot,
23462374
DAG.getIntPtrConstant(PartOffset, DL));
23472375
MemOpChains.push_back(
23482376
DAG.getStore(Chain, DL, PartValue, Address,
23492377
MachinePointerInfo::getFixedStack(MF, FI)));
2378+
assert(PartOffset && "Offset should be non-zero.");
23502379
assert((PartOffset + PartValue.getValueType().getStoreSize() <=
23512380
SlotVT.getStoreSize()) && "Not enough space for argument part!");
2352-
++I;
23532381
}
23542382
ArgValue = SpillSlot;
23552383
} else
@@ -2534,7 +2562,7 @@ bool SystemZTargetLowering::CanLowerReturn(
25342562
// Special case that we cannot easily detect in RetCC_SystemZ since
25352563
// i128 may not be a legal type.
25362564
for (auto &Out : Outs)
2537-
if (Out.ArgVT == MVT::i128)
2565+
if (Out.ArgVT.isScalarInteger() && Out.ArgVT.getSizeInBits() > 64)
25382566
return false;
25392567

25402568
SmallVector<CCValAssign, 16> RetLocs;

0 commit comments

Comments
 (0)