[llvm] ed98063 - [InstCombine] Make worklist check in memcpy from constant fold more precise

[Nikita Popov via llvm-commits]

Author: Nikita Popov
Date: 2023-01-23T14:15:41+01:00
New Revision: ed9806363beb1caf5265e75a64de904b60218093

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

LOG: [InstCombine] Make worklist check in memcpy from constant fold more precise

The phi operands need to be either in the worklist or be the
alloca itself, because that one does not require replacement.

Added: 
    

Modified: 
    llvm/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp
    llvm/test/Transforms/InstCombine/ptr-replace-alloca.ll

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp b/llvm/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp
index abe5ad3382bf1..71efe0f3391f1 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp
@@ -259,25 +259,30 @@ namespace {
 // instruction.
 class PointerReplacer {
 public:
-  PointerReplacer(InstCombinerImpl &IC) : IC(IC) {}
+  PointerReplacer(InstCombinerImpl &IC, Instruction &Root)
+    : IC(IC), Root(Root) {}
 
-  bool collectUsers(Instruction &I);
-  void replacePointer(Instruction &I, Value *V);
+  bool collectUsers();
+  void replacePointer(Value *V);
 
 private:
   bool collectUsersRecursive(Instruction &I);
   void replace(Instruction *I);
   Value *getReplacement(Value *I);
+  bool isAvailable(Instruction *I) const {
+    return I == &Root || Worklist.contains(I);
+  }
 
   SmallPtrSet<Instruction *, 32> ValuesToRevisit;
   SmallSetVector<Instruction *, 4> Worklist;
   MapVector<Value *, Value *> WorkMap;
   InstCombinerImpl &IC;
+  Instruction &Root;
 };
 } // end anonymous namespace
 
-bool PointerReplacer::collectUsers(Instruction &I) {
-  if (!collectUsersRecursive(I))
+bool PointerReplacer::collectUsers() {
+  if (!collectUsersRecursive(Root))
     return false;
 
   // Ensure that all outstanding (indirect) users of I
@@ -306,7 +311,7 @@ bool PointerReplacer::collectUsersRecursive(Instruction &I) {
       // store the PHI for revisiting and skip this iteration of the
       // loop.
       if (any_of(PHI->incoming_values(), [this](Value *V) {
-            return !Worklist.contains(cast<Instruction>(V));
+            return !isAvailable(cast<Instruction>(V));
           })) {
         ValuesToRevisit.insert(Inst);
         continue;
@@ -406,13 +411,13 @@ void PointerReplacer::replace(Instruction *I) {
   }
 }
 
-void PointerReplacer::replacePointer(Instruction &I, Value *V) {
+void PointerReplacer::replacePointer(Value *V) {
 #ifndef NDEBUG
-  auto *PT = cast<PointerType>(I.getType());
+  auto *PT = cast<PointerType>(Root.getType());
   auto *NT = cast<PointerType>(V->getType());
   assert(PT != NT && PT->hasSameElementTypeAs(NT) && "Invalid usage");
 #endif
-  WorkMap[&I] = V;
+  WorkMap[&Root] = V;
 
   for (Instruction *Workitem : Worklist)
     replace(Workitem);
@@ -493,13 +498,13 @@ Instruction *InstCombinerImpl::visitAllocaInst(AllocaInst &AI) {
         return NewI;
       }
 
-      PointerReplacer PtrReplacer(*this);
-      if (PtrReplacer.collectUsers(AI)) {
+      PointerReplacer PtrReplacer(*this, AI);
+      if (PtrReplacer.collectUsers()) {
         for (Instruction *Delete : ToDelete)
           eraseInstFromFunction(*Delete);
 
         Value *Cast = Builder.CreateBitCast(TheSrc, DestTy);
-        PtrReplacer.replacePointer(AI, Cast);
+        PtrReplacer.replacePointer(Cast);
         ++NumGlobalCopies;
       }
     }

diff  --git a/llvm/test/Transforms/InstCombine/ptr-replace-alloca.ll b/llvm/test/Transforms/InstCombine/ptr-replace-alloca.ll
index de21d2e9b357f..7821eb1491074 100644
--- a/llvm/test/Transforms/InstCombine/ptr-replace-alloca.ll
+++ b/llvm/test/Transforms/InstCombine/ptr-replace-alloca.ll
@@ -284,15 +284,12 @@ join:
 define i32 @addrspace_
diff _remove_alloca(i1 %cond) {
 ; CHECK-LABEL: @addrspace_
diff _remove_alloca(
 ; CHECK-NEXT:  entry:
-; CHECK-NEXT:    [[A:%.*]] = alloca [32 x i8], align 1
-; CHECK-NEXT:    call void @llvm.memcpy.p0.p1.i64(ptr noundef nonnull align 1 dereferenceable(32) [[A]], ptr addrspace(1) noundef align 16 dereferenceable(32) @g2, i64 32, i1 false)
-; CHECK-NEXT:    [[GEP:%.*]] = getelementptr inbounds [32 x i8], ptr [[A]], i64 0, i64 2
 ; CHECK-NEXT:    br i1 [[COND:%.*]], label [[IF:%.*]], label [[JOIN:%.*]]
 ; CHECK:       if:
 ; CHECK-NEXT:    br label [[JOIN]]
 ; CHECK:       join:
-; CHECK-NEXT:    [[PHI:%.*]] = phi ptr [ [[A]], [[IF]] ], [ [[GEP]], [[ENTRY:%.*]] ]
-; CHECK-NEXT:    [[V:%.*]] = load i32, ptr [[PHI]], align 4
+; CHECK-NEXT:    [[PHI1:%.*]] = phi ptr addrspace(1) [ @g2, [[IF]] ], [ getelementptr inbounds ([32 x i8], ptr addrspace(1) @g2, i64 0, i64 2), [[ENTRY:%.*]] ]
+; CHECK-NEXT:    [[V:%.*]] = load i32, ptr addrspace(1) [[PHI1]], align 4
 ; CHECK-NEXT:    ret i32 [[V]]
 ;
 entry: