[llvm] d8b253b - [SelectionDAG] Mark frame index as "aliased" at argument copy elison (#89712)

[via llvm-commits]

Author: Björn Pettersson
Date: 2024-04-23T13:49:18+02:00
New Revision: d8b253be56b3e9073b3e59123cf2da0bcde20c63

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

LOG: [SelectionDAG] Mark frame index as "aliased" at argument copy elison (#89712)

This is a fix for miscompiles reported in
  https://github.com/llvm/llvm-project/issues/89060

After argument copy elison the IR value for the eliminated alloca
is aliasing with the fixed stack object. This patch is making sure
that we mark the fixed stack object as being aliased with IR values
to avoid that for example schedulers are reordering accesses to
the fixed stack object. This could otherwise happen when there is a
mix of MemOperands refering the shared fixed stack slow via both
the IR value for the elided alloca, and via a fixed stack pseudo
source value (as would be the case when lowering the arguments).

Added: 
    

Modified: 
    llvm/include/llvm/CodeGen/MachineFrameInfo.h
    llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
    llvm/test/CodeGen/Hexagon/arg-copy-elison.ll

Removed: 
    


################################################################################
diff  --git a/llvm/include/llvm/CodeGen/MachineFrameInfo.h b/llvm/include/llvm/CodeGen/MachineFrameInfo.h
index 0fe73fec7ee67f..a2c78e9e093d01 100644
--- a/llvm/include/llvm/CodeGen/MachineFrameInfo.h
+++ b/llvm/include/llvm/CodeGen/MachineFrameInfo.h
@@ -701,6 +701,13 @@ class MachineFrameInfo {
     return Objects[ObjectIdx+NumFixedObjects].isAliased;
   }
 
+  /// Set "maybe pointed to by an LLVM IR value" for an object.
+  void setIsAliasedObjectIndex(int ObjectIdx, bool IsAliased) {
+    assert(unsigned(ObjectIdx+NumFixedObjects) < Objects.size() &&
+           "Invalid Object Idx!");
+    Objects[ObjectIdx+NumFixedObjects].isAliased = IsAliased;
+  }
+
   /// Returns true if the specified index corresponds to an immutable object.
   bool isImmutableObjectIndex(int ObjectIdx) const {
     // Tail calling functions can clobber their function arguments.

diff  --git a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
index 3f69f7ad54477e..319465865fb1d1 100644
--- a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
@@ -11128,7 +11128,7 @@ static void tryToElideArgumentCopy(
   }
 
   // Perform the elision. Delete the old stack object and replace its only use
-  // in the variable info map. Mark the stack object as mutable.
+  // in the variable info map. Mark the stack object as mutable and aliased.
   LLVM_DEBUG({
     dbgs() << "Eliding argument copy from " << Arg << " to " << *AI << '\n'
            << "  Replacing frame index " << OldIndex << " with " << FixedIndex
@@ -11136,6 +11136,7 @@ static void tryToElideArgumentCopy(
   });
   MFI.RemoveStackObject(OldIndex);
   MFI.setIsImmutableObjectIndex(FixedIndex, false);
+  MFI.setIsAliasedObjectIndex(FixedIndex, true);
   AllocaIndex = FixedIndex;
   ArgCopyElisionFrameIndexMap.insert({OldIndex, FixedIndex});
   for (SDValue ArgVal : ArgVals)

diff  --git a/llvm/test/CodeGen/Hexagon/arg-copy-elison.ll b/llvm/test/CodeGen/Hexagon/arg-copy-elison.ll
index 14f61f7de351a7..f0c30c301f446f 100644
--- a/llvm/test/CodeGen/Hexagon/arg-copy-elison.ll
+++ b/llvm/test/CodeGen/Hexagon/arg-copy-elison.ll
@@ -18,14 +18,12 @@ define i32 @f(i32, i32, i32, i32, i32, i32, i32, i32, i32, i32, i32, i32, i32, i
 ; CHECK-NEXT:  // %bb.0:
 ; CHECK-NEXT:    {
 ; CHECK-NEXT:     r0 = memw(r29+#36)
-; CHECK-NEXT:     memw(r29+#32) = ##666
-; CHECK-NEXT:    }
-; CHECK-NEXT:    {
 ; CHECK-NEXT:     r1 = memw(r29+#28)
-; CHECK-NEXT:     r2 = memw(r29+#32)
 ; CHECK-NEXT:    }
 ; CHECK-NEXT:    {
 ; CHECK-NEXT:     r0 = sub(r1,r0)
+; CHECK-NEXT:     r2 = memw(r29+#32)
+; CHECK-NEXT:     memw(r29+#32) = ##666
 ; CHECK-NEXT:    }
 ; CHECK-NEXT:    {
 ; CHECK-NEXT:     r0 = xor(r0,r2)