Skip to content

Commit

Permalink
[SystemZ] Assign the full space for promoted and split outgoing args.
Browse files Browse the repository at this point in the history
When a large "irregular" (e.g. i96) integer call argument is converted to
indirect, 64-bit parts are stored to the stack. The full stack space
(e.g. i128) was not allocated prior to this patch, but rather just the exact
space of the original type. This caused neighboring values on the stack to be
overwritten.

Thanks to Josh Stone for reporting this.

Review: Ulrich Weigand
Fixes https://bugs.llvm.org/show_bug.cgi?id=49322
Differential Revision: https://reviews.llvm.org/D97514
  • Loading branch information
JonPsson committed Mar 2, 2021
1 parent 95540f9 commit 52bbbf4
Show file tree
Hide file tree
Showing 2 changed files with 72 additions and 4 deletions.
22 changes: 18 additions & 4 deletions llvm/lib/Target/SystemZ/SystemZISelLowering.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1547,6 +1547,7 @@ SystemZTargetLowering::LowerCall(CallLoweringInfo &CLI,
bool IsVarArg = CLI.IsVarArg;
MachineFunction &MF = DAG.getMachineFunction();
EVT PtrVT = getPointerTy(MF.getDataLayout());
LLVMContext &Ctx = *DAG.getContext();

// Detect unsupported vector argument and return types.
if (Subtarget.hasVector()) {
Expand All @@ -1556,7 +1557,7 @@ SystemZTargetLowering::LowerCall(CallLoweringInfo &CLI,

// Analyze the operands of the call, assigning locations to each operand.
SmallVector<CCValAssign, 16> ArgLocs;
SystemZCCState ArgCCInfo(CallConv, IsVarArg, MF, ArgLocs, *DAG.getContext());
SystemZCCState ArgCCInfo(CallConv, IsVarArg, MF, ArgLocs, Ctx);
ArgCCInfo.AnalyzeCallOperands(Outs, CC_SystemZ);

// We don't support GuaranteedTailCallOpt, only automatically-detected
Expand All @@ -1581,14 +1582,25 @@ SystemZTargetLowering::LowerCall(CallLoweringInfo &CLI,

if (VA.getLocInfo() == CCValAssign::Indirect) {
// Store the argument in a stack slot and pass its address.
SDValue SpillSlot = DAG.CreateStackTemporary(Outs[I].ArgVT);
unsigned ArgIndex = Outs[I].OrigArgIndex;
EVT SlotVT;
if (I + 1 != E && Outs[I + 1].OrigArgIndex == ArgIndex) {
// Allocate the full stack space for a promoted (and split) argument.
Type *OrigArgType = CLI.Args[Outs[I].OrigArgIndex].Ty;
EVT OrigArgVT = getValueType(MF.getDataLayout(), OrigArgType);
MVT PartVT = getRegisterTypeForCallingConv(Ctx, CLI.CallConv, OrigArgVT);
unsigned N = getNumRegistersForCallingConv(Ctx, CLI.CallConv, OrigArgVT);
SlotVT = EVT::getIntegerVT(Ctx, PartVT.getSizeInBits() * N);
} else {
SlotVT = Outs[I].ArgVT;
}
SDValue SpillSlot = DAG.CreateStackTemporary(SlotVT);
int FI = cast<FrameIndexSDNode>(SpillSlot)->getIndex();
MemOpChains.push_back(
DAG.getStore(Chain, DL, ArgValue, SpillSlot,
MachinePointerInfo::getFixedStack(MF, FI)));
// If the original argument was split (e.g. i128), we need
// to store all parts of it here (and pass just one address).
unsigned ArgIndex = Outs[I].OrigArgIndex;
assert (Outs[I].PartOffset == 0);
while (I + 1 != E && Outs[I + 1].OrigArgIndex == ArgIndex) {
SDValue PartValue = OutVals[I + 1];
Expand All @@ -1598,6 +1610,8 @@ SystemZTargetLowering::LowerCall(CallLoweringInfo &CLI,
MemOpChains.push_back(
DAG.getStore(Chain, DL, PartValue, Address,
MachinePointerInfo::getFixedStack(MF, FI)));
assert((PartOffset + PartValue.getValueType().getStoreSize() <=
SlotVT.getStoreSize()) && "Not enough space for argument part!");
++I;
}
ArgValue = SpillSlot;
Expand Down Expand Up @@ -1691,7 +1705,7 @@ SystemZTargetLowering::LowerCall(CallLoweringInfo &CLI,

// Assign locations to each value returned by this call.
SmallVector<CCValAssign, 16> RetLocs;
CCState RetCCInfo(CallConv, IsVarArg, MF, RetLocs, *DAG.getContext());
CCState RetCCInfo(CallConv, IsVarArg, MF, RetLocs, Ctx);
RetCCInfo.AnalyzeCallResult(Ins, RetCC_SystemZ);

// Copy all of the result registers out of their specified physreg.
Expand Down
54 changes: 54 additions & 0 deletions llvm/test/CodeGen/SystemZ/args-11.ll
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
; Test outgoing promoted arguments that are split (and passed by reference).
;
; RUN: llc < %s -mtriple=s390x-linux-gnu | FileCheck %s

; The i96 arg is promoted to i128 and should get the full stack space.
declare void @fn1(i96)
define i32 @fn2() {
; CHECK-LABEL: fn2:
; CHECK: # %bb.0:
; CHECK-NEXT: stmg %r14, %r15, 112(%r15)
; CHECK-NEXT: .cfi_offset %r14, -48
; CHECK-NEXT: .cfi_offset %r15, -40
; CHECK-NEXT: aghi %r15, -184
; CHECK-NEXT: .cfi_def_cfa_offset 344
; CHECK-NEXT: mvhi 180(%r15), -1
; CHECK-NEXT: mvghi 168(%r15), 0
; CHECK-NEXT: la %r2, 160(%r15)
; CHECK-NEXT: mvghi 160(%r15), 0
; CHECK-NEXT: brasl %r14, fn1@PLT
; CHECK-NEXT: l %r2, 180(%r15)
; CHECK-NEXT: lmg %r14, %r15, 296(%r15)
; CHECK-NEXT: br %r14
%1 = alloca i32
store i32 -1, i32* %1
call void @fn1(i96 0)
%2 = load i32, i32* %1
ret i32 %2
}

declare void @fn3(i136)
define i32 @fn4() {
; CHECK-LABEL: fn4:
; CHECK: # %bb.0:
; CHECK-NEXT: stmg %r14, %r15, 112(%r15)
; CHECK-NEXT: .cfi_offset %r14, -48
; CHECK-NEXT: .cfi_offset %r15, -40
; CHECK-NEXT: aghi %r15, -192
; CHECK-NEXT: .cfi_def_cfa_offset 352
; CHECK-NEXT: mvhi 188(%r15), -1
; CHECK-NEXT: mvghi 176(%r15), 0
; CHECK-NEXT: mvghi 168(%r15), 0
; CHECK-NEXT: la %r2, 160(%r15)
; CHECK-NEXT: mvghi 160(%r15), 0
; CHECK-NEXT: brasl %r14, fn3@PLT
; CHECK-NEXT: l %r2, 188(%r15)
; CHECK-NEXT: lmg %r14, %r15, 304(%r15)
; CHECK-NEXT: br %r14
%1 = alloca i32
store i32 -1, i32* %1
call void @fn3(i136 0)
%2 = load i32, i32* %1
ret i32 %2
}

0 comments on commit 52bbbf4

Please sign in to comment.