Skip to content

Commit f147d67

Browse files
authored
[X86] Use StackArgTokenFactor for all stores when setting up tail calls. (#126244)
Before this patch, the stack argument token factor was used if any outgoing stack slots needed to be written, but not when writing the return address to a stack slot. Writing the return address stack slot can also alias a stack slot for an input argument. Always use StackArgumentTokenFactor, to ensure the store of the return address will always use it, i.e. happen after all input arguments are loaded. PR: #126244
1 parent d7fd2a2 commit f147d67

File tree

2 files changed

+12
-12
lines changed

2 files changed

+12
-12
lines changed

llvm/lib/Target/X86/X86ISelLoweringCall.cpp

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -2326,12 +2326,13 @@ X86TargetLowering::LowerCall(TargetLowering::CallLoweringInfo &CLI,
23262326
// shuffling arguments passed in memory.
23272327
if (!IsSibcall && isTailCall) {
23282328
// Force all the incoming stack arguments to be loaded from the stack
2329-
// before any new outgoing arguments are stored to the stack, because the
2330-
// outgoing stack slots may alias the incoming argument stack slots, and
2331-
// the alias isn't otherwise explicit. This is slightly more conservative
2332-
// than necessary, because it means that each store effectively depends
2333-
// on every argument instead of just those arguments it would clobber.
2334-
SDValue ArgChain = DAG.getStackArgumentTokenFactor(Chain);
2329+
// before any new outgoing arguments or the return address are stored to the
2330+
// stack, because the outgoing stack slots may alias the incoming argument
2331+
// stack slots, and the alias isn't otherwise explicit. This is slightly
2332+
// more conservative than necessary, because it means that each store
2333+
// effectively depends on every argument instead of just those arguments it
2334+
// would clobber.
2335+
Chain = DAG.getStackArgumentTokenFactor(Chain);
23352336

23362337
SmallVector<SDValue, 8> MemOpChains2;
23372338
SDValue FIN;
@@ -2373,13 +2374,12 @@ X86TargetLowering::LowerCall(TargetLowering::CallLoweringInfo &CLI,
23732374
Source = DAG.getNode(ISD::ADD, dl, getPointerTy(DAG.getDataLayout()),
23742375
StackPtr, Source);
23752376

2376-
MemOpChains2.push_back(CreateCopyOfByValArgument(Source, FIN,
2377-
ArgChain,
2378-
Flags, DAG, dl));
2377+
MemOpChains2.push_back(
2378+
CreateCopyOfByValArgument(Source, FIN, Chain, Flags, DAG, dl));
23792379
} else {
23802380
// Store relative to framepointer.
23812381
MemOpChains2.push_back(DAG.getStore(
2382-
ArgChain, dl, Arg, FIN,
2382+
Chain, dl, Arg, FIN,
23832383
MachinePointerInfo::getFixedStack(DAG.getMachineFunction(), FI)));
23842384
}
23852385
}

llvm/test/CodeGen/X86/swifttailcc-store-ret-address-aliasing-stack-slot.ll

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,9 +24,9 @@ define swifttailcc void @test(ptr %0, ptr swiftasync %1, i64 %2, i64 %3, ptr %4,
2424
; CHECK-NEXT: movq {{[0-9]+}}(%rsp), %r15
2525
; CHECK-NEXT: callq _foo
2626
; CHECK-NEXT: movq %r14, (%rax)
27+
; CHECK-NEXT: movl [[OFF:[0-9]+]](%rsp), %edx
2728
; CHECK-NEXT: movq {{[0-9]+}}(%rsp), %rcx
28-
; CHECK-NEXT: movq %rcx, [[OFF:[0-9]+]](%rsp)
29-
; CHECK-NEXT: movl [[OFF]](%rsp), %edx
29+
; CHECK-NEXT: movq %rcx, [[OFF]](%rsp)
3030
; CHECK-NEXT: movq %rax, %r14
3131
; CHECK-NEXT: movq %r13, %rdi
3232
; CHECK-NEXT: movq %r15, %rsi

0 commit comments

Comments
 (0)