<div dir="ltr">Yes along with r<span style="font-size:12.8px">277693.</span></div><div class="gmail_extra"><br><div class="gmail_quote">On Thu, Aug 4, 2016 at 10:13 AM, Hans Wennborg <span dir="ltr"><<a href="mailto:hans@chromium.org" target="_blank">hans@chromium.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Should we merge this to 3.9?<br>
<div class="HOEnZb"><div class="h5"><br>
On Wed, Aug 3, 2016 at 9:24 PM, David Majnemer via llvm-commits<br>
<<a href="mailto:llvm-commits@lists.llvm.org">llvm-commits@lists.llvm.org</a>> wrote:<br>
> Author: majnemer<br>
> Date: Wed Aug  3 23:24:02 2016<br>
> New Revision: 277691<br>
><br>
> URL: <a href="http://llvm.org/viewvc/llvm-project?rev=277691&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project?rev=277691&view=rev</a><br>
> Log:<br>
> Reinstate "[CloneFunction] Don't remove side effecting calls"<br>
><br>
> This reinstates r277611 + r277614 and reverts r277642.  A cast_or_null<br>
> should have been a dyn_cast_or_null.<br>
><br>
> Modified:<br>
>     llvm/trunk/lib/Analysis/<wbr>InstructionSimplify.cpp<br>
>     llvm/trunk/lib/Transforms/<wbr>Utils/CloneFunction.cpp<br>
>     llvm/trunk/test/Transforms/<wbr>Inline/inline_constprop.ll<br>
><br>
> Modified: llvm/trunk/lib/Analysis/<wbr>InstructionSimplify.cpp<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Analysis/InstructionSimplify.cpp?rev=277691&r1=277690&r2=277691&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project/llvm/trunk/lib/<wbr>Analysis/InstructionSimplify.<wbr>cpp?rev=277691&r1=277690&r2=<wbr>277691&view=diff</a><br>
> ==============================<wbr>==============================<wbr>==================<br>
> --- llvm/trunk/lib/Analysis/<wbr>InstructionSimplify.cpp (original)<br>
> +++ llvm/trunk/lib/Analysis/<wbr>InstructionSimplify.cpp Wed Aug  3 23:24:02 2016<br>
> @@ -4356,7 +4356,8 @@ static bool replaceAndRecursivelySimplif<br>
><br>
>      // Gracefully handle edge cases where the instruction is not wired into any<br>
>      // parent block.<br>
> -    if (I->getParent())<br>
> +    if (I->getParent() && !I->isEHPad() && !isa<TerminatorInst>(I) &&<br>
> +        !I->mayHaveSideEffects())<br>
>        I->eraseFromParent();<br>
>    } else {<br>
>      Worklist.insert(I);<br>
> @@ -4384,7 +4385,8 @@ static bool replaceAndRecursivelySimplif<br>
><br>
>      // Gracefully handle edge cases where the instruction is not wired into any<br>
>      // parent block.<br>
> -    if (I->getParent())<br>
> +    if (I->getParent() && !I->isEHPad() && !isa<TerminatorInst>(I) &&<br>
> +        !I->mayHaveSideEffects())<br>
>        I->eraseFromParent();<br>
>    }<br>
>    return Simplified;<br>
><br>
> Modified: llvm/trunk/lib/Transforms/<wbr>Utils/CloneFunction.cpp<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Utils/CloneFunction.cpp?rev=277691&r1=277690&r2=277691&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project/llvm/trunk/lib/<wbr>Transforms/Utils/<wbr>CloneFunction.cpp?rev=277691&<wbr>r1=277690&r2=277691&view=diff</a><br>
> ==============================<wbr>==============================<wbr>==================<br>
> --- llvm/trunk/lib/Transforms/<wbr>Utils/CloneFunction.cpp (original)<br>
> +++ llvm/trunk/lib/Transforms/<wbr>Utils/CloneFunction.cpp Wed Aug  3 23:24:02 2016<br>
> @@ -14,6 +14,7 @@<br>
>  //===-------------------------<wbr>------------------------------<wbr>---------------===//<br>
><br>
>  #include "llvm/Transforms/Utils/<wbr>Cloning.h"<br>
> +#include "llvm/ADT/SetVector.h"<br>
>  #include "llvm/ADT/SmallVector.h"<br>
>  #include "llvm/Analysis/<wbr>ConstantFolding.h"<br>
>  #include "llvm/Analysis/<wbr>InstructionSimplify.h"<br>
> @@ -552,9 +553,39 @@ void llvm::<wbr>CloneAndPruneIntoFromInst(Fun<br>
>    // two PHINodes, the iteration over the old PHIs remains valid, and the<br>
>    // mapping will just map us to the new node (which may not even be a PHI<br>
>    // node).<br>
> +  const DataLayout &DL = NewFunc->getParent()-><wbr>getDataLayout();<br>
> +  SmallSetVector<const Value *, 8> Worklist;<br>
>    for (unsigned Idx = 0, Size = PHIToResolve.size(); Idx != Size; ++Idx)<br>
> -    if (PHINode *PN = dyn_cast<PHINode>(VMap[<wbr>PHIToResolve[Idx]]))<br>
> -      recursivelySimplifyInstruction<wbr>(PN);<br>
> +    if (isa<PHINode>(VMap[<wbr>PHIToResolve[Idx]]))<br>
> +      Worklist.insert(PHIToResolve[<wbr>Idx]);<br>
> +<br>
> +  // Note that we must test the size on each iteration, the worklist can grow.<br>
> +  for (unsigned Idx = 0; Idx != Worklist.size(); ++Idx) {<br>
> +    const Value *OrigV = Worklist[Idx];<br>
> +    auto *I = cast_or_null<Instruction>(<wbr>VMap.lookup(OrigV));<br>
> +    if (!I)<br>
> +      continue;<br>
> +<br>
> +    // See if this instruction simplifies.<br>
> +    Value *SimpleV = SimplifyInstruction(I, DL);<br>
> +    if (!SimpleV)<br>
> +      continue;<br>
> +<br>
> +    // Stash away all the uses of the old instruction so we can check them for<br>
> +    // recursive simplifications after a RAUW. This is cheaper than checking all<br>
> +    // uses of To on the recursive step in most cases.<br>
> +    for (const User *U : OrigV->users())<br>
> +      Worklist.insert(cast<<wbr>Instruction>(U));<br>
> +<br>
> +    // Replace the instruction with its simplified value.<br>
> +    I->replaceAllUsesWith(SimpleV)<wbr>;<br>
> +<br>
> +    // If the original instruction had no side effects, remove it.<br>
> +    if (isInstructionTriviallyDead(I)<wbr>)<br>
> +      I->eraseFromParent();<br>
> +    else<br>
> +      VMap[OrigV] = I;<br>
> +  }<br>
><br>
>    // Now that the inlined function body has been fully constructed, go through<br>
>    // and zap unconditional fall-through branches. This happens all the time when<br>
><br>
> Modified: llvm/trunk/test/Transforms/<wbr>Inline/inline_constprop.ll<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/Inline/inline_constprop.ll?rev=277691&r1=277690&r2=277691&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project/llvm/trunk/test/<wbr>Transforms/Inline/inline_<wbr>constprop.ll?rev=277691&r1=<wbr>277690&r2=277691&view=diff</a><br>
> ==============================<wbr>==============================<wbr>==================<br>
> --- llvm/trunk/test/Transforms/<wbr>Inline/inline_constprop.ll (original)<br>
> +++ llvm/trunk/test/Transforms/<wbr>Inline/inline_constprop.ll Wed Aug  3 23:24:02 2016<br>
> @@ -279,3 +279,25 @@ return:<br>
>    %retval.0 = phi i32* [ %b, %if.end3 ], [ %a, %if.then ]<br>
>    ret i32* %retval.0<br>
>  }<br>
> +<br>
> +declare i32 @PR28802.external(i32 returned %p1)<br>
> +<br>
> +define internal i32 @PR28802.callee() {<br>
> +entry:<br>
> +  br label %cont<br>
> +<br>
> +cont:<br>
> +  %0 = phi i32 [ 0, %entry ]<br>
> +  %call = call i32 @PR28802.external(i32 %0)<br>
> +  ret i32 %call<br>
> +}<br>
> +<br>
> +define i32 @PR28802() {<br>
> +entry:<br>
> +  %call = call i32 @PR28802.callee()<br>
> +  ret i32 %call<br>
> +}<br>
> +<br>
> +; CHECK-LABEL: define i32 @PR28802(<br>
> +; CHECK: call i32 @PR28802.external(i32 0)<br>
> +; CHECK: ret i32 0<br>
><br>
><br>
> ______________________________<wbr>_________________<br>
> llvm-commits mailing list<br>
> <a href="mailto:llvm-commits@lists.llvm.org">llvm-commits@lists.llvm.org</a><br>
> <a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/<wbr>mailman/listinfo/llvm-commits</a><br>
</div></div></blockquote></div><br></div>