[llvm] 81ec494 - [SDAGBuilder] Handle multi-part arguments in argument copy elision (PR63430)

[Nikita Popov via llvm-commits]

Author: Nikita Popov
Date: 2023-06-22T17:04:56+02:00
New Revision: 81ec494c363d4934e692e8b35e0b3fbbc3de1c2b

URL: https://github.com/llvm/llvm-project/commit/81ec494c363d4934e692e8b35e0b3fbbc3de1c2b
DIFF: https://github.com/llvm/llvm-project/commit/81ec494c363d4934e692e8b35e0b3fbbc3de1c2b.diff

LOG: [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 https://github.com/llvm/llvm-project/issues/63430.

Added: 
    

Modified: 
    llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
    llvm/test/CodeGen/X86/pr63430.ll

Removed: 
    


################################################################################
diff  --git a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
index d70de26655b95..550ad3f2ddddf 100644
--- a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
@@ -10616,9 +10616,9 @@ static void tryToElideArgumentCopy(
     DenseMap<int, int> &ArgCopyElisionFrameIndexMap,
     SmallPtrSetImpl<const Instruction *> &ElidedArgCopyInstrs,
     ArgCopyElisionMapTy &ArgCopyElisionCandidates, const Argument &Arg,
-    SDValue ArgVal, bool &ArgHasUses) {
+    ArrayRef<SDValue> ArgVals, bool &ArgHasUses) {
   // Check if this is a load from a fixed stack object.
-  auto *LNode = dyn_cast<LoadSDNode>(ArgVal);
+  auto *LNode = dyn_cast<LoadSDNode>(ArgVals[0]);
   if (!LNode)
     return;
   auto *FINode = dyn_cast<FrameIndexSDNode>(LNode->getBasePtr().getNode());
@@ -10661,7 +10661,8 @@ static void tryToElideArgumentCopy(
   MFI.setIsImmutableObjectIndex(FixedIndex, false);
   AllocaIndex = FixedIndex;
   ArgCopyElisionFrameIndexMap.insert({OldIndex, FixedIndex});
-  Chains.push_back(ArgVal.getValue(1));
+  for (SDValue ArgVal : ArgVals)
+    Chains.push_back(ArgVal.getValue(1));
 
   // Avoid emitting code for the store implementing the copy.
   const StoreInst *SI = ArgCopyIter->second.second;
@@ -10922,9 +10923,14 @@ void SelectionDAGISel::LowerArguments(const Function &F) {
     // Elide the copying store if the target loaded this argument from a
     // suitable fixed stack object.
     if (Ins[i].Flags.isCopyElisionCandidate()) {
+      unsigned NumParts = 0;
+      for (EVT VT : ValueVTs)
+        NumParts += TLI->getNumRegistersForCallingConv(*CurDAG->getContext(),
+                                                       F.getCallingConv(), VT);
+
       tryToElideArgumentCopy(*FuncInfo, Chains, ArgCopyElisionFrameIndexMap,
                              ElidedArgCopyInstrs, ArgCopyElisionCandidates, Arg,
-                             InVals[i], ArgHasUses);
+                             ArrayRef(&InVals[i], NumParts), ArgHasUses);
     }
 
     // If this argument is unused then remember its value. It is used to generate

diff  --git a/llvm/test/CodeGen/X86/pr63430.ll b/llvm/test/CodeGen/X86/pr63430.ll
index 5e9c627f01ff8..24d650d02be80 100644
--- a/llvm/test/CodeGen/X86/pr63430.ll
+++ b/llvm/test/CodeGen/X86/pr63430.ll
@@ -1,14 +1,14 @@
 ; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py UTC_ARGS: --no_x86_scrub_sp --version 2
 ; RUN: llc -mtriple=x86_64-unknown-linux < %s | FileCheck %s
 
-; TODO: This is a miscompile.
+; Make sure the argument is read before the stack slot is over-written.
 define i1 @test(ptr %a0, ptr %a1, ptr %a2, ptr %a3, ptr %a4, ptr %a5, i128 %x) {
 ; CHECK-LABEL: test:
 ; CHECK:       # %bb.0:
 ; CHECK-NEXT:    movq 8(%rsp), %rax
 ; CHECK-NEXT:    xorps %xmm0, %xmm0
-; CHECK-NEXT:    movaps %xmm0, 8(%rsp)
 ; CHECK-NEXT:    andq 16(%rsp), %rax
+; CHECK-NEXT:    movaps %xmm0, 8(%rsp)
 ; CHECK-NEXT:    cmpq $-1, %rax
 ; CHECK-NEXT:    sete %al
 ; CHECK-NEXT:    retq