[llvm] 5fa14ee - [MemCpyOpt] Don't hoist above producer of pointer operand

[Nikita Popov via llvm-commits]

Author: Nikita Popov
Date: 2022-10-05T14:52:33+02:00
New Revision: 5fa14ee835223995b9df67985c30341a01e1e01b

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

LOG: [MemCpyOpt] Don't hoist above producer of pointer operand

This was already handled correctly below, but not checked for the
original store pointer operand. Encountered when converting tests
to opaque pointers, where the intermediate bitcast goes away.

Added: 
    

Modified: 
    llvm/lib/Transforms/Scalar/MemCpyOptimizer.cpp
    llvm/test/Transforms/MemCpyOpt/aggregate-type-crash.ll

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Transforms/Scalar/MemCpyOptimizer.cpp b/llvm/lib/Transforms/Scalar/MemCpyOptimizer.cpp
index f53f3eb8da754..6785a8421511f 100644
--- a/llvm/lib/Transforms/Scalar/MemCpyOptimizer.cpp
+++ b/llvm/lib/Transforms/Scalar/MemCpyOptimizer.cpp
@@ -546,9 +546,17 @@ bool MemCpyOptPass::moveUp(StoreInst *SI, Instruction *P, const LoadInst *LI) {
   // Keep track of the arguments of all instruction we plan to lift
   // so we can make sure to lift them as well if appropriate.
   DenseSet<Instruction*> Args;
-  if (auto *Ptr = dyn_cast<Instruction>(SI->getPointerOperand()))
-    if (Ptr->getParent() == SI->getParent())
-      Args.insert(Ptr);
+  auto AddArg = [&](Value *Arg) {
+    auto *I = dyn_cast<Instruction>(Arg);
+    if (I && I->getParent() == SI->getParent()) {
+      // Cannot hoist user of P above P
+      if (I == P) return false;
+      Args.insert(I);
+    }
+    return true;
+  };
+  if (!AddArg(SI->getPointerOperand()))
+    return false;
 
   // Instruction to lift before P.
   SmallVector<Instruction *, 8> ToLift{SI};
@@ -612,14 +620,9 @@ bool MemCpyOptPass::moveUp(StoreInst *SI, Instruction *P, const LoadInst *LI) {
     }
 
     ToLift.push_back(C);
-    for (unsigned k = 0, e = C->getNumOperands(); k != e; ++k)
-      if (auto *A = dyn_cast<Instruction>(C->getOperand(k))) {
-        if (A->getParent() == SI->getParent()) {
-          // Cannot hoist user of P above P
-          if(A == P) return false;
-          Args.insert(A);
-        }
-      }
+    for (Value *Op : C->operands())
+      if (!AddArg(Op))
+        return false;
   }
 
   // Find MSSA insertion point. Normally P will always have a corresponding

diff  --git a/llvm/test/Transforms/MemCpyOpt/aggregate-type-crash.ll b/llvm/test/Transforms/MemCpyOpt/aggregate-type-crash.ll
index 01a5f5f218404..1f05eedd825ec 100644
--- a/llvm/test/Transforms/MemCpyOpt/aggregate-type-crash.ll
+++ b/llvm/test/Transforms/MemCpyOpt/aggregate-type-crash.ll
@@ -7,22 +7,20 @@ target triple = "x86_64-apple-macosx10.14.0"
 %my_struct = type { i8, i32 }
 
 ; Function Attrs: inaccessiblemem_or_argmemonly
-declare noalias i8* @my_malloc(%my_struct*) #0
+declare noalias ptr @my_malloc(ptr) #0
 
-define void @my_func(%my_struct* %0) {
+define void @my_func(ptr %0) {
 ; CHECK-LABEL: @my_func(
 ; CHECK-NEXT:  entry:
-; CHECK-NEXT:    [[TMP1:%.*]] = load [[MY_STRUCT:%.*]], %my_struct* [[TMP0:%.*]], align 4
-; CHECK-NEXT:    [[TMP2:%.*]] = call i8* @my_malloc(%my_struct* [[TMP0]])
-; CHECK-NEXT:    [[TMP3:%.*]] = bitcast i8* [[TMP2]] to %my_struct*
-; CHECK-NEXT:    store [[MY_STRUCT]] [[TMP1]], %my_struct* [[TMP3]], align 4
+; CHECK-NEXT:    [[TMP1:%.*]] = load [[MY_STRUCT:%.*]], ptr [[TMP0:%.*]], align 4
+; CHECK-NEXT:    [[TMP2:%.*]] = call ptr @my_malloc(ptr [[TMP0]])
+; CHECK-NEXT:    store [[MY_STRUCT]] [[TMP1]], ptr [[TMP2]], align 4
 ; CHECK-NEXT:    ret void
 ;
 entry:
-  %1 = load %my_struct, %my_struct* %0
-  %2 = call i8* @my_malloc(%my_struct* %0)
-  %3 = bitcast i8* %2 to %my_struct*
-  store %my_struct %1, %my_struct* %3
+  %1 = load %my_struct, ptr %0
+  %2 = call ptr @my_malloc(ptr %0)
+  store %my_struct %1, ptr %2
   ret void
 }