[llvm] r277611 - [CloneFunction] Don't remove side effecting calls

[David Majnemer via llvm-commits]

Author: majnemer
Date: Wed Aug  3 12:12:47 2016
New Revision: 277611

URL: http://llvm.org/viewvc/llvm-project?rev=277611&view=rev
Log:
[CloneFunction] Don't remove side effecting calls

We were able to figure out that the result of a call is some constant.
While propagating that fact, we added the constant to the value map.
This is problematic because it results in us losing the call site when
processing the value map.

This fixes PR28802.

Modified:
    llvm/trunk/lib/Analysis/InstructionSimplify.cpp
    llvm/trunk/lib/Transforms/Utils/CloneFunction.cpp
    llvm/trunk/test/Transforms/Inline/inline_constprop.ll

Modified: llvm/trunk/lib/Analysis/InstructionSimplify.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Analysis/InstructionSimplify.cpp?rev=277611&r1=277610&r2=277611&view=diff
==============================================================================
--- llvm/trunk/lib/Analysis/InstructionSimplify.cpp (original)
+++ llvm/trunk/lib/Analysis/InstructionSimplify.cpp Wed Aug  3 12:12:47 2016
@@ -4356,7 +4356,8 @@ static bool replaceAndRecursivelySimplif
 
     // Gracefully handle edge cases where the instruction is not wired into any
     // parent block.
-    if (I->getParent())
+    if (I->getParent() && !I->isEHPad() && !isa<TerminatorInst>(I) &&
+        !I->mayHaveSideEffects())
       I->eraseFromParent();
   } else {
     Worklist.insert(I);
@@ -4384,7 +4385,8 @@ static bool replaceAndRecursivelySimplif
 
     // Gracefully handle edge cases where the instruction is not wired into any
     // parent block.
-    if (I->getParent())
+    if (I->getParent() && !I->isEHPad() && !isa<TerminatorInst>(I) &&
+        !I->mayHaveSideEffects())
       I->eraseFromParent();
   }
   return Simplified;

Modified: llvm/trunk/lib/Transforms/Utils/CloneFunction.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Utils/CloneFunction.cpp?rev=277611&r1=277610&r2=277611&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/Utils/CloneFunction.cpp (original)
+++ llvm/trunk/lib/Transforms/Utils/CloneFunction.cpp Wed Aug  3 12:12:47 2016
@@ -14,6 +14,7 @@
 //===----------------------------------------------------------------------===//
 
 #include "llvm/Transforms/Utils/Cloning.h"
+#include "llvm/ADT/SetVector.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/Analysis/ConstantFolding.h"
 #include "llvm/Analysis/InstructionSimplify.h"
@@ -552,9 +553,39 @@ void llvm::CloneAndPruneIntoFromInst(Fun
   // two PHINodes, the iteration over the old PHIs remains valid, and the
   // mapping will just map us to the new node (which may not even be a PHI
   // node).
+  const DataLayout &DL = NewFunc->getParent()->getDataLayout();
+  SmallSetVector<const Value *, 8> Worklist;
   for (unsigned Idx = 0, Size = PHIToResolve.size(); Idx != Size; ++Idx)
-    if (PHINode *PN = dyn_cast<PHINode>(VMap[PHIToResolve[Idx]]))
-      recursivelySimplifyInstruction(PN);
+    if (isa<PHINode>(VMap[PHIToResolve[Idx]]))
+      Worklist.insert(PHIToResolve[Idx]);
+
+  // Note that we must test the size on each iteration, the worklist can grow.
+  for (unsigned Idx = 0; Idx != Worklist.size(); ++Idx) {
+    const Value *OrigV = Worklist[Idx];
+    auto *I = cast<Instruction>(VMap.lookup(OrigV));
+    if (!I)
+      continue;
+
+    // See if this instruction simplifies.
+    Value *SimpleV = SimplifyInstruction(I, DL);
+    if (!SimpleV)
+      continue;
+
+    // Stash away all the uses of the old instruction so we can check them for
+    // recursive simplifications after a RAUW. This is cheaper than checking all
+    // uses of To on the recursive step in most cases.
+    for (const User *U : OrigV->users())
+      Worklist.insert(cast<Instruction>(U));
+
+    // Replace the instruction with its simplified value.
+    I->replaceAllUsesWith(SimpleV);
+
+    // If the original instruction had no side effects, remove it.
+    if (isInstructionTriviallyDead(I))
+      I->eraseFromParent();
+    else
+      VMap[OrigV] = I;
+  }
 
   // Now that the inlined function body has been fully constructed, go through
   // and zap unconditional fall-through branches. This happens all the time when

Modified: llvm/trunk/test/Transforms/Inline/inline_constprop.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/Inline/inline_constprop.ll?rev=277611&r1=277610&r2=277611&view=diff
==============================================================================
--- llvm/trunk/test/Transforms/Inline/inline_constprop.ll (original)
+++ llvm/trunk/test/Transforms/Inline/inline_constprop.ll Wed Aug  3 12:12:47 2016
@@ -279,3 +279,25 @@ return:
   %retval.0 = phi i32* [ %b, %if.end3 ], [ %a, %if.then ]
   ret i32* %retval.0
 }
+
+declare i32 @PR28802.external(i32 returned %p1)
+
+define internal i32 @PR28802.callee() {
+entry:
+  br label %cont
+
+cont:
+  %0 = phi i32 [ 0, %entry ]
+  %call = call i32 @PR28802.external(i32 %0)
+  ret i32 %call
+}
+
+define i32 @PR28802() {
+entry:
+  %call = call i32 @PR28802.callee()
+  ret i32 %call
+}
+
+; CHECK-LABEL: define i32 @PR28802(
+; CHECK: call i32 @PR28802.external(i32 0)
+; CHECK: ret i32 0