Skip to content

Commit 81ec494

Browse files
committed
[SDAGBuilder] Handle multi-part arguments in argument copy elision (PR63430)
When eliding an argument copy, we need to update the chain to ensure the argument reads are performed before later writes. However, the code doing this only handled this for the first part of the argument. If the argument had multiple parts, the chains of the later parts were dropped. Make sure we preserve all chains. Fixes #63430.
1 parent d179421 commit 81ec494

File tree

2 files changed

+12
-6
lines changed

2 files changed

+12
-6
lines changed

llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10616,9 +10616,9 @@ static void tryToElideArgumentCopy(
1061610616
DenseMap<int, int> &ArgCopyElisionFrameIndexMap,
1061710617
SmallPtrSetImpl<const Instruction *> &ElidedArgCopyInstrs,
1061810618
ArgCopyElisionMapTy &ArgCopyElisionCandidates, const Argument &Arg,
10619-
SDValue ArgVal, bool &ArgHasUses) {
10619+
ArrayRef<SDValue> ArgVals, bool &ArgHasUses) {
1062010620
// Check if this is a load from a fixed stack object.
10621-
auto *LNode = dyn_cast<LoadSDNode>(ArgVal);
10621+
auto *LNode = dyn_cast<LoadSDNode>(ArgVals[0]);
1062210622
if (!LNode)
1062310623
return;
1062410624
auto *FINode = dyn_cast<FrameIndexSDNode>(LNode->getBasePtr().getNode());
@@ -10661,7 +10661,8 @@ static void tryToElideArgumentCopy(
1066110661
MFI.setIsImmutableObjectIndex(FixedIndex, false);
1066210662
AllocaIndex = FixedIndex;
1066310663
ArgCopyElisionFrameIndexMap.insert({OldIndex, FixedIndex});
10664-
Chains.push_back(ArgVal.getValue(1));
10664+
for (SDValue ArgVal : ArgVals)
10665+
Chains.push_back(ArgVal.getValue(1));
1066510666

1066610667
// Avoid emitting code for the store implementing the copy.
1066710668
const StoreInst *SI = ArgCopyIter->second.second;
@@ -10922,9 +10923,14 @@ void SelectionDAGISel::LowerArguments(const Function &F) {
1092210923
// Elide the copying store if the target loaded this argument from a
1092310924
// suitable fixed stack object.
1092410925
if (Ins[i].Flags.isCopyElisionCandidate()) {
10926+
unsigned NumParts = 0;
10927+
for (EVT VT : ValueVTs)
10928+
NumParts += TLI->getNumRegistersForCallingConv(*CurDAG->getContext(),
10929+
F.getCallingConv(), VT);
10930+
1092510931
tryToElideArgumentCopy(*FuncInfo, Chains, ArgCopyElisionFrameIndexMap,
1092610932
ElidedArgCopyInstrs, ArgCopyElisionCandidates, Arg,
10927-
InVals[i], ArgHasUses);
10933+
ArrayRef(&InVals[i], NumParts), ArgHasUses);
1092810934
}
1092910935

1093010936
// If this argument is unused then remember its value. It is used to generate

llvm/test/CodeGen/X86/pr63430.ll

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,14 @@
11
; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py UTC_ARGS: --no_x86_scrub_sp --version 2
22
; RUN: llc -mtriple=x86_64-unknown-linux < %s | FileCheck %s
33

4-
; TODO: This is a miscompile.
4+
; Make sure the argument is read before the stack slot is over-written.
55
define i1 @test(ptr %a0, ptr %a1, ptr %a2, ptr %a3, ptr %a4, ptr %a5, i128 %x) {
66
; CHECK-LABEL: test:
77
; CHECK: # %bb.0:
88
; CHECK-NEXT: movq 8(%rsp), %rax
99
; CHECK-NEXT: xorps %xmm0, %xmm0
10-
; CHECK-NEXT: movaps %xmm0, 8(%rsp)
1110
; CHECK-NEXT: andq 16(%rsp), %rax
11+
; CHECK-NEXT: movaps %xmm0, 8(%rsp)
1212
; CHECK-NEXT: cmpq $-1, %rax
1313
; CHECK-NEXT: sete %al
1414
; CHECK-NEXT: retq

0 commit comments

Comments
 (0)