From bd7189d76c58fb03733058d6ea65745e54c7233a Mon Sep 17 00:00:00 2001 From: Tim Northover Date: Fri, 23 Oct 2015 20:30:02 +0000 Subject: [PATCH] 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 --- lib/Transforms/Scalar/GVN.cpp | 14 ++++++++----- test/Transforms/GVN/pre-gep-load.ll | 31 +++++++++++++++++++++++++++++ 2 files changed, 40 insertions(+), 5 deletions(-) diff --git a/lib/Transforms/Scalar/GVN.cpp b/lib/Transforms/Scalar/GVN.cpp index d91540cc26e..e2822e320a8 100644 --- 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 291af359a7a..a46dc22ade8 100644 --- 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) -- 2.34.1