[llvm] 0e6deaa - [InstCombine] Add Visited set to isOnlyCopiedFromConstantMemory()

[Nikita Popov via llvm-commits]

Author: Nikita Popov
Date: 2023-01-11T17:12:26+01:00
New Revision: 0e6deaa2a062e43200aab9cb5e8a76e754441bd9

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

LOG: [InstCombine] Add Visited set to isOnlyCopiedFromConstantMemory()

I don't think this matters right now (because InstCombine cleans
up unreachable code early), but this will help to make sure that
we don't infinite loop once we handle phi nodes. The added test
is an example where this would happen.

Added: 
    

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

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp b/llvm/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp
index 474b4e788dee6..3feeeaf0063b4 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp
@@ -46,10 +46,16 @@ isOnlyCopiedFromConstantMemory(AAResults *AA, AllocaInst *V,
   // ahead and replace the value with the memory location, this lets the caller
   // quickly eliminate the markers.
 
-  SmallVector<PointerIntPair<Value *, 1, bool>, 35> ValuesToInspect;
-  ValuesToInspect.emplace_back(V, false);
-  while (!ValuesToInspect.empty()) {
-    const auto [Value, IsOffset] = ValuesToInspect.pop_back_val();
+  using ValueAndIsOffset = PointerIntPair<Value *, 1, bool>;
+  SmallVector<ValueAndIsOffset, 32> Worklist;
+  SmallPtrSet<ValueAndIsOffset, 32> Visited;
+  Worklist.emplace_back(V, false);
+  while (!Worklist.empty()) {
+    ValueAndIsOffset Elem = Worklist.pop_back_val();
+    if (!Visited.insert(Elem).second)
+      continue;
+
+    const auto [Value, IsOffset] = Elem;
     for (auto &U : Value->uses()) {
       auto *I = cast<Instruction>(U.getUser());
 
@@ -61,13 +67,13 @@ isOnlyCopiedFromConstantMemory(AAResults *AA, AllocaInst *V,
 
       if (isa<BitCastInst>(I) || isa<AddrSpaceCastInst>(I)) {
         // If uses of the bitcast are ok, we are ok.
-        ValuesToInspect.emplace_back(I, IsOffset);
+        Worklist.emplace_back(I, IsOffset);
         continue;
       }
       if (auto *GEP = dyn_cast<GetElementPtrInst>(I)) {
         // If the GEP has all zero indices, it doesn't offset the pointer. If it
         // doesn't, it does.
-        ValuesToInspect.emplace_back(I, IsOffset || !GEP->hasAllZeroIndices());
+        Worklist.emplace_back(I, IsOffset || !GEP->hasAllZeroIndices());
         continue;
       }
 

diff  --git a/llvm/test/Transforms/InstCombine/replace-alloca-phi.ll b/llvm/test/Transforms/InstCombine/replace-alloca-phi.ll
index bc33dd41e63a4..15baf865100bf 100644
--- a/llvm/test/Transforms/InstCombine/replace-alloca-phi.ll
+++ b/llvm/test/Transforms/InstCombine/replace-alloca-phi.ll
@@ -266,6 +266,35 @@ join:
   ret i32 %v
 }
 
+define i32 @phi_loop(i1 %c) {
+; CHECK-LABEL: @phi_loop(
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:    [[ALLOCA:%.*]] = alloca [32 x i8], align 1
+; CHECK-NEXT:    call void @llvm.memcpy.p0.p0.i64(ptr noundef nonnull align 1 dereferenceable(32) [[ALLOCA]], ptr noundef nonnull align 16 dereferenceable(32) @g1, i64 32, i1 false)
+; CHECK-NEXT:    br label [[LOOP:%.*]]
+; CHECK:       loop:
+; CHECK-NEXT:    [[PTR:%.*]] = phi ptr [ [[ALLOCA]], [[ENTRY:%.*]] ], [ [[PTR_NEXT:%.*]], [[LOOP]] ]
+; CHECK-NEXT:    [[PTR_NEXT]] = getelementptr i8, ptr [[PTR]], i64 4
+; CHECK-NEXT:    br i1 [[C:%.*]], label [[EXIT:%.*]], label [[LOOP]]
+; CHECK:       exit:
+; CHECK-NEXT:    [[V:%.*]] = load i32, ptr [[PTR]], align 4
+; CHECK-NEXT:    ret i32 [[V]]
+;
+entry:
+  %alloca = alloca [32 x i8]
+  call void @llvm.memcpy.p0.p0.i64(ptr %alloca, ptr @g1, i64 32, i1 false)
+  br label %loop
+
+loop:
+  %ptr = phi ptr [ %alloca, %entry ], [ %ptr.next, %loop ]
+  %ptr.next = getelementptr i8, ptr %ptr, i64 4
+  br i1 %c, label %exit, label %loop
+
+exit:
+  %v = load i32, ptr %ptr
+  ret i32 %v
+}
+
 declare void @llvm.memcpy.p1i8.p0i8.i64(ptr addrspace(1), ptr, i64, i1)
 declare void @llvm.memcpy.p0.p0.i64(ptr, ptr, i64, i1)
 declare void @llvm.memcpy.p0.p1.i64(ptr, ptr addrspace(1), i64, i1)