GVN: don't try to replace instruction with itself.

After some look-ahead PRE was added for GEPs, an instruction could end
up in the table of candidates before it was actually inspected. When
this happened the pass might decide it was the best candidate to
replace itself. This didn't go well.

Should fix PR25291

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@251145 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/lib/Transforms/Scalar/GVN.cpp b/lib/Transforms/Scalar/GVN.cpp
index d91540cc26e10fae4e1530a0aabee4e3b262b224..e2822e320a851e7896d1b85a15908c1fa15c9e49 100644 (file)
--- a/lib/Transforms/Scalar/GVN.cpp
+++ b/lib/Transforms/Scalar/GVN.cpp
@@ -2387,17 +2387,21 @@ bool GVN::processInstruction(Instruction *I) {
 
   // Perform fast-path value-number based elimination of values inherited from
   // dominators.
-  Value *repl = findLeader(I->getParent(), Num);
-  if (!repl) {
+  Value *Repl = findLeader(I->getParent(), Num);
+  if (!Repl) {
     // Failure, just remember this instance for future use.
     addToLeaderTable(Num, I, I->getParent());
     return false;
+  } else if (Repl == I) {
+    // If I was the result of a shortcut PRE, it might already be in the table
+    // and the best replacement for itself. Nothing to do.
+    return false;
   }
 
   // Remove it!
-  patchAndReplaceAllUsesWith(I, repl);
-  if (MD && repl->getType()->getScalarType()->isPointerTy())
-    MD->invalidateCachedPointerInfo(repl);
+  patchAndReplaceAllUsesWith(I, Repl);
+  if (MD && Repl->getType()->getScalarType()->isPointerTy())
+    MD->invalidateCachedPointerInfo(Repl);
   markInstructionForDeletion(I);
   return true;
 }

diff --git a/test/Transforms/GVN/pre-gep-load.ll b/test/Transforms/GVN/pre-gep-load.ll
index 291af359a7a134a3170d7f42a05f5b7dd4e16e47..a46dc22ade89ed30dbe613aebb0a8189fb760ac5 100644 (file)
--- a/test/Transforms/GVN/pre-gep-load.ll
+++ b/test/Transforms/GVN/pre-gep-load.ll
@@ -47,3 +47,34 @@ return:                                           ; preds = %sw.default, %sw.bb2
   %retval.0 = phi double [ 0.000000e+00, %sw.default ], [ %sub6, %sw.bb2 ], [ %sub, %if.then ]
   ret double %retval.0
 }
+
+; The load causes the GEP's operands to be PREd earlier than normal. The
+; resulting sext ends up in pre.dest and in the GVN system before that BB is
+; actually processed. Make sure we can deal with the situation.
+
+define void @test_shortcut_safe(i1 %tst, i32 %p1, i32* %a) {
+; CHECK-LABEL: define void @test_shortcut_safe
+; CHECK: [[SEXT1:%.*]] = sext i32 %p1 to i64
+; CHECK: [[PHI1:%.*]] = phi i64 [ [[SEXT1]], {{%.*}} ], [ [[PHI2:%.*]], {{%.*}} ]
+; CHECK: [[SEXT2:%.*]] = sext i32 %p1 to i64
+; CHECK: [[PHI2]] = phi i64 [ [[SEXT2]], {{.*}} ], [ [[PHI1]], {{%.*}} ]
+; CHECK: getelementptr inbounds i32, i32* %a, i64 [[PHI2]]
+
+  br i1 %tst, label %sext1, label %pre.dest
+
+pre.dest:
+  br label %sext.use
+
+sext1:
+  %idxprom = sext i32 %p1 to i64
+  br label %sext.use
+
+sext.use:
+  %idxprom2 = sext i32 %p1 to i64
+  %arrayidx3 = getelementptr inbounds i32, i32* %a, i64 %idxprom2
+  %val = load i32, i32* %arrayidx3, align 4
+  tail call void (i32) @g(i32 %val)
+  br label %pre.dest
+}
+
+declare void @g(i32)