[llvm] r277691 - Reinstate "[CloneFunction] Don't remove side effecting calls"

[Hans Wennborg via llvm-commits]

Should we merge this to 3.9?

On Wed, Aug 3, 2016 at 9:24 PM, David Majnemer via llvm-commits
<llvm-commits at lists.llvm.org> wrote:
> Author: majnemer
> Date: Wed Aug  3 23:24:02 2016
> New Revision: 277691
>
> URL: http://llvm.org/viewvc/llvm-project?rev=277691&view=rev
> Log:
> Reinstate "[CloneFunction] Don't remove side effecting calls"
>
> This reinstates r277611 + r277614 and reverts r277642.  A cast_or_null
> should have been a dyn_cast_or_null.
>
> 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=277691&r1=277690&r2=277691&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Analysis/InstructionSimplify.cpp (original)
> +++ llvm/trunk/lib/Analysis/InstructionSimplify.cpp Wed Aug  3 23:24:02 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=277691&r1=277690&r2=277691&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Transforms/Utils/CloneFunction.cpp (original)
> +++ llvm/trunk/lib/Transforms/Utils/CloneFunction.cpp Wed Aug  3 23:24:02 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_or_null<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=277691&r1=277690&r2=277691&view=diff
> ==============================================================================
> --- llvm/trunk/test/Transforms/Inline/inline_constprop.ll (original)
> +++ llvm/trunk/test/Transforms/Inline/inline_constprop.ll Wed Aug  3 23:24:02 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
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits