[llvm] r279647 - [SCCP] Don't delete side-effecting instructions

[Sanjoy Das via llvm-commits]

Author: sanjoy
Date: Wed Aug 24 13:10:21 2016
New Revision: 279647

URL: http://llvm.org/viewvc/llvm-project?rev=279647&view=rev
Log:
[SCCP] Don't delete side-effecting instructions

I'm not sure if the `!isa<CallInst>(Inst) &&
!isa<TerminatorInst>(Inst))` bit is correct either, but this fixes the
case we know is broken.

Modified:
    llvm/trunk/lib/Transforms/Scalar/SCCP.cpp
    llvm/trunk/test/Transforms/SCCP/calltest.ll

Modified: llvm/trunk/lib/Transforms/Scalar/SCCP.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/SCCP.cpp?rev=279647&r1=279646&r2=279647&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/Scalar/SCCP.cpp (original)
+++ llvm/trunk/lib/Transforms/Scalar/SCCP.cpp Wed Aug 24 13:10:21 2016
@@ -1559,17 +1559,6 @@ static bool tryToReplaceWithConstant(SCC
   return true;
 }
 
-static bool tryToReplaceInstWithConstant(SCCPSolver &Solver, Instruction *Inst,
-                                         bool shouldEraseFromParent) {
-  if (!tryToReplaceWithConstant(Solver, Inst))
-    return false;
-
-  // Delete the instruction.
-  if (shouldEraseFromParent)
-    Inst->eraseFromParent();
-  return true;
-}
-
 // runSCCP() - Run the Sparse Conditional Constant Propagation algorithm,
 // and return true if the function was modified.
 //
@@ -1618,8 +1607,9 @@ static bool runSCCP(Function &F, const D
       if (Inst->getType()->isVoidTy() || isa<TerminatorInst>(Inst))
         continue;
 
-      if (tryToReplaceInstWithConstant(Solver, Inst,
-                                       true /* shouldEraseFromParent */)) {
+      if (tryToReplaceWithConstant(Solver, Inst)) {
+        if (isInstructionTriviallyDead(Inst))
+          Inst->eraseFromParent();
         // Hey, we just changed something!
         MadeChanges = true;
         ++NumInstRemoved;
@@ -1823,10 +1813,9 @@ static bool runIPSCCP(Module &M, const D
         Instruction *Inst = &*BI++;
         if (Inst->getType()->isVoidTy())
           continue;
-        if (tryToReplaceInstWithConstant(
-                Solver, Inst,
-                !isa<CallInst>(Inst) &&
-                    !isa<TerminatorInst>(Inst) /* shouldEraseFromParent */)) {
+        if (tryToReplaceWithConstant(Solver, Inst)) {
+          if (!isa<CallInst>(Inst) && !isa<TerminatorInst>(Inst))
+            Inst->eraseFromParent();
           // Hey, we just changed something!
           MadeChanges = true;
           ++IPNumInstRemoved;

Modified: llvm/trunk/test/Transforms/SCCP/calltest.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/SCCP/calltest.ll?rev=279647&r1=279646&r2=279647&view=diff
==============================================================================
--- llvm/trunk/test/Transforms/SCCP/calltest.ll (original)
+++ llvm/trunk/test/Transforms/SCCP/calltest.ll Wed Aug 24 13:10:21 2016
@@ -1,12 +1,16 @@
-; RUN: opt < %s -sccp -loop-deletion -simplifycfg -S | not grep br
+; RUN: opt < %s -sccp -loop-deletion -simplifycfg -S | FileCheck %s
 
+declare double @sqrt(double) readnone nounwind
+%empty = type {}
+declare %empty @has_side_effects()
+
+define double @test_0(i32 %param) {
+; CHECK-LABEL: @test_0(
+; CHECK-NOT: br
+entry:
 ; No matter how hard you try, sqrt(1.0) is always 1.0.  This allows the
 ; optimizer to delete this loop.
 
-declare double @sqrt(double)
-
-define double @test(i32 %param) {
-entry:
 	br label %Loop
 Loop:		; preds = %Loop, %entry
 	%I2 = phi i32 [ 0, %entry ], [ %I3, %Loop ]		; <i32> [#uses=1]
@@ -19,3 +23,9 @@ Exit:		; preds = %Loop
 	ret double %V
 }
 
+define i32 @test_1() {
+; CHECK-LABEL: @test_1(
+; CHECK: call %empty @has_side_effects()
+  %1 = call %empty @has_side_effects()
+  ret i32 0
+}