[llvm] r251145 - GVN: don't try to replace instruction with itself.

[Tim Northover via llvm-commits]

Author: tnorthover
Date: Fri Oct 23 15:30:02 2015
New Revision: 251145

URL: http://llvm.org/viewvc/llvm-project?rev=251145&view=rev
Log:
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

Modified:
    llvm/trunk/lib/Transforms/Scalar/GVN.cpp
    llvm/trunk/test/Transforms/GVN/pre-gep-load.ll

Modified: llvm/trunk/lib/Transforms/Scalar/GVN.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/GVN.cpp?rev=251145&r1=251144&r2=251145&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/Scalar/GVN.cpp (original)
+++ llvm/trunk/lib/Transforms/Scalar/GVN.cpp Fri Oct 23 15:30:02 2015
@@ -2387,17 +2387,21 @@ bool GVN::processInstruction(Instruction
 
   // 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;
 }

Modified: llvm/trunk/test/Transforms/GVN/pre-gep-load.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/GVN/pre-gep-load.ll?rev=251145&r1=251144&r2=251145&view=diff
==============================================================================
--- llvm/trunk/test/Transforms/GVN/pre-gep-load.ll (original)
+++ llvm/trunk/test/Transforms/GVN/pre-gep-load.ll Fri Oct 23 15:30:02 2015
@@ -47,3 +47,34 @@ return:
   %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)