<div dir="ltr"><div>r275756 reverted the ability for function-attrs to infer returned. I fixed function-attrs with r276008 which exposed a latent bug in users of ConstantFoldInstruction.<br></div><div><br></div><div>However, similar bugs were found and fixed in r273711 which is before the branch point. I'd consider r276389 to be a more complete version of that earlier fix.</div><div><br></div><div>Having said all that, I think the right thing to do is to merge it into 3.9</div></div><div class="gmail_extra"><br><div class="gmail_quote">On Fri, Jul 22, 2016 at 7:06 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 this be merge to 3.9?<br>
<br>
Thanks,<br>
Hans<br>
<div class="HOEnZb"><div class="h5"><br>
On Fri, Jul 22, 2016 at 12:54 AM, 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: Thu Jul 21 23:54:44 2016<br>
> New Revision: 276389<br>
><br>
> URL: <a href="http://llvm.org/viewvc/llvm-project?rev=276389&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=276389&view=rev</a><br>
> Log:<br>
> Don't remove side effecting instructions due to ConstantFoldInstruction<br>
><br>
> Just because we can constant fold the result of an instruction does not<br>
> imply that we can delete the instruction. It may have side effects.<br>
><br>
> This fixes PR28655.<br>
><br>
> Modified:<br>
> llvm/trunk/lib/Transforms/IPO/GlobalOpt.cpp<br>
> llvm/trunk/lib/Transforms/InstCombine/InstructionCombining.cpp<br>
> llvm/trunk/lib/Transforms/Scalar/ConstantProp.cpp<br>
> llvm/trunk/lib/Transforms/Scalar/JumpThreading.cpp<br>
> llvm/trunk/lib/Transforms/Scalar/LICM.cpp<br>
> llvm/trunk/test/Transforms/ConstProp/calls.ll<br>
> llvm/trunk/test/Transforms/InstCombine/call.ll<br>
> llvm/trunk/test/Transforms/InstCombine/log-pow.ll<br>
><br>
> Modified: llvm/trunk/lib/Transforms/IPO/GlobalOpt.cpp<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/IPO/GlobalOpt.cpp?rev=276389&r1=276388&r2=276389&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/IPO/GlobalOpt.cpp?rev=276389&r1=276388&r2=276389&view=diff</a><br>
> ==============================================================================<br>
> --- llvm/trunk/lib/Transforms/IPO/GlobalOpt.cpp (original)<br>
> +++ llvm/trunk/lib/Transforms/IPO/GlobalOpt.cpp Thu Jul 21 23:54:44 2016<br>
> @@ -44,6 +44,7 @@<br>
> #include "llvm/Transforms/Utils/CtorUtils.h"<br>
> #include "llvm/Transforms/Utils/Evaluator.h"<br>
> #include "llvm/Transforms/Utils/GlobalStatus.h"<br>
> +#include "llvm/Transforms/Utils/Local.h"<br>
> #include <algorithm><br>
> using namespace llvm;<br>
><br>
> @@ -779,7 +780,8 @@ static void ConstantPropUsersOf(Value *V<br>
> // Instructions could multiply use V.<br>
> while (UI != E && *UI == I)<br>
> ++UI;<br>
> - I->eraseFromParent();<br>
> + if (isInstructionTriviallyDead(I, TLI))<br>
> + I->eraseFromParent();<br>
> }<br>
> }<br>
><br>
><br>
> Modified: llvm/trunk/lib/Transforms/InstCombine/InstructionCombining.cpp<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/InstCombine/InstructionCombining.cpp?rev=276389&r1=276388&r2=276389&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/InstCombine/InstructionCombining.cpp?rev=276389&r1=276388&r2=276389&view=diff</a><br>
> ==============================================================================<br>
> --- llvm/trunk/lib/Transforms/InstCombine/InstructionCombining.cpp (original)<br>
> +++ llvm/trunk/lib/Transforms/InstCombine/InstructionCombining.cpp Thu Jul 21 23:54:44 2016<br>
> @@ -2830,7 +2830,8 @@ bool InstCombiner::run() {<br>
> // Add operands to the worklist.<br>
> replaceInstUsesWith(*I, C);<br>
> ++NumConstProp;<br>
> - eraseInstFromFunction(*I);<br>
> + if (isInstructionTriviallyDead(I, TLI))<br>
> + eraseInstFromFunction(*I);<br>
> MadeIRChange = true;<br>
> continue;<br>
> }<br>
> @@ -2851,7 +2852,8 @@ bool InstCombiner::run() {<br>
> // Add operands to the worklist.<br>
> replaceInstUsesWith(*I, C);<br>
> ++NumConstProp;<br>
> - eraseInstFromFunction(*I);<br>
> + if (isInstructionTriviallyDead(I, TLI))<br>
> + eraseInstFromFunction(*I);<br>
> MadeIRChange = true;<br>
> continue;<br>
> }<br>
> @@ -3007,7 +3009,8 @@ static bool AddReachableCodeToWorklist(B<br>
> << *Inst << '\n');<br>
> Inst->replaceAllUsesWith(C);<br>
> ++NumConstProp;<br>
> - Inst->eraseFromParent();<br>
> + if (isInstructionTriviallyDead(Inst, TLI))<br>
> + Inst->eraseFromParent();<br>
> continue;<br>
> }<br>
><br>
><br>
> Modified: llvm/trunk/lib/Transforms/Scalar/ConstantProp.cpp<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/ConstantProp.cpp?rev=276389&r1=276388&r2=276389&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/ConstantProp.cpp?rev=276389&r1=276388&r2=276389&view=diff</a><br>
> ==============================================================================<br>
> --- llvm/trunk/lib/Transforms/Scalar/ConstantProp.cpp (original)<br>
> +++ llvm/trunk/lib/Transforms/Scalar/ConstantProp.cpp Thu Jul 21 23:54:44 2016<br>
> @@ -19,6 +19,7 @@<br>
> //===----------------------------------------------------------------------===//<br>
><br>
> #include "llvm/Transforms/Scalar.h"<br>
> +#include "llvm/Transforms/Utils/Local.h"<br>
> #include "llvm/ADT/Statistic.h"<br>
> #include "llvm/Analysis/ConstantFolding.h"<br>
> #include "llvm/IR/Constant.h"<br>
> @@ -90,11 +91,13 @@ bool ConstantPropagation::runOnFunction(<br>
><br>
> // Remove the dead instruction.<br>
> WorkList.erase(I);<br>
> - I->eraseFromParent();<br>
> + if (isInstructionTriviallyDead(I, TLI)) {<br>
> + I->eraseFromParent();<br>
> + ++NumInstKilled;<br>
> + }<br>
><br>
> // We made a change to the function...<br>
> Changed = true;<br>
> - ++NumInstKilled;<br>
> }<br>
> }<br>
> return Changed;<br>
><br>
> Modified: llvm/trunk/lib/Transforms/Scalar/JumpThreading.cpp<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/JumpThreading.cpp?rev=276389&r1=276388&r2=276389&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/JumpThreading.cpp?rev=276389&r1=276388&r2=276389&view=diff</a><br>
> ==============================================================================<br>
> --- llvm/trunk/lib/Transforms/Scalar/JumpThreading.cpp (original)<br>
> +++ llvm/trunk/lib/Transforms/Scalar/JumpThreading.cpp Thu Jul 21 23:54:44 2016<br>
> @@ -758,7 +758,8 @@ bool JumpThreadingPass::ProcessBlock(Bas<br>
> ConstantFoldInstruction(I, BB->getModule()->getDataLayout(), TLI);<br>
> if (SimpleVal) {<br>
> I->replaceAllUsesWith(SimpleVal);<br>
> - I->eraseFromParent();<br>
> + if (isInstructionTriviallyDead(I, TLI))<br>
> + I->eraseFromParent();<br>
> Condition = SimpleVal;<br>
> }<br>
> }<br>
><br>
> Modified: llvm/trunk/lib/Transforms/Scalar/LICM.cpp<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/LICM.cpp?rev=276389&r1=276388&r2=276389&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/LICM.cpp?rev=276389&r1=276388&r2=276389&view=diff</a><br>
> ==============================================================================<br>
> --- llvm/trunk/lib/Transforms/Scalar/LICM.cpp (original)<br>
> +++ llvm/trunk/lib/Transforms/Scalar/LICM.cpp Thu Jul 21 23:54:44 2016<br>
> @@ -377,9 +377,11 @@ bool llvm::hoistRegion(DomTreeNode *N, A<br>
> &I, I.getModule()->getDataLayout(), TLI)) {<br>
> DEBUG(dbgs() << "LICM folding inst: " << I << " --> " << *C << '\n');<br>
> CurAST->copyValue(&I, C);<br>
> - CurAST->deleteValue(&I);<br>
> I.replaceAllUsesWith(C);<br>
> - I.eraseFromParent();<br>
> + if (isInstructionTriviallyDead(&I, TLI)) {<br>
> + CurAST->deleteValue(&I);<br>
> + I.eraseFromParent();<br>
> + }<br>
> continue;<br>
> }<br>
><br>
><br>
> Modified: llvm/trunk/test/Transforms/ConstProp/calls.ll<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/ConstProp/calls.ll?rev=276389&r1=276388&r2=276389&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/ConstProp/calls.ll?rev=276389&r1=276388&r2=276389&view=diff</a><br>
> ==============================================================================<br>
> --- llvm/trunk/test/Transforms/ConstProp/calls.ll (original)<br>
> +++ llvm/trunk/test/Transforms/ConstProp/calls.ll Thu Jul 21 23:54:44 2016<br>
> @@ -1,47 +1,47 @@<br>
> ; RUN: opt < %s -constprop -S | FileCheck %s<br>
> ; RUN: opt < %s -constprop -disable-simplify-libcalls -S | FileCheck %s --check-prefix=FNOBUILTIN<br>
><br>
> -declare double @acos(double)<br>
> -declare double @asin(double)<br>
> -declare double @atan(double)<br>
> -declare double @atan2(double, double)<br>
> -declare double @ceil(double)<br>
> -declare double @cos(double)<br>
> -declare double @cosh(double)<br>
> -declare double @exp(double)<br>
> -declare double @exp2(double)<br>
> -declare double @fabs(double)<br>
> -declare double @floor(double)<br>
> -declare double @fmod(double, double)<br>
> -declare double @log(double)<br>
> -declare double @log10(double)<br>
> -declare double @pow(double, double)<br>
> -declare double @sin(double)<br>
> -declare double @sinh(double)<br>
> -declare double @sqrt(double)<br>
> -declare double @tan(double)<br>
> -declare double @tanh(double)<br>
> +declare double @acos(double) readnone nounwind<br>
> +declare double @asin(double) readnone nounwind<br>
> +declare double @atan(double) readnone nounwind<br>
> +declare double @atan2(double, double) readnone nounwind<br>
> +declare double @ceil(double) readnone nounwind<br>
> +declare double @cos(double) readnone nounwind<br>
> +declare double @cosh(double) readnone nounwind<br>
> +declare double @exp(double) readnone nounwind<br>
> +declare double @exp2(double) readnone nounwind<br>
> +declare double @fabs(double) readnone nounwind<br>
> +declare double @floor(double) readnone nounwind<br>
> +declare double @fmod(double, double) readnone nounwind<br>
> +declare double @log(double) readnone nounwind<br>
> +declare double @log10(double) readnone nounwind<br>
> +declare double @pow(double, double) readnone nounwind<br>
> +declare double @sin(double) readnone nounwind<br>
> +declare double @sinh(double) readnone nounwind<br>
> +declare double @sqrt(double) readnone nounwind<br>
> +declare double @tan(double) readnone nounwind<br>
> +declare double @tanh(double) readnone nounwind<br>
><br>
> -declare float @acosf(float)<br>
> -declare float @asinf(float)<br>
> -declare float @atanf(float)<br>
> -declare float @atan2f(float, float)<br>
> -declare float @ceilf(float)<br>
> -declare float @cosf(float)<br>
> -declare float @coshf(float)<br>
> -declare float @expf(float)<br>
> -declare float @exp2f(float)<br>
> -declare float @fabsf(float)<br>
> -declare float @floorf(float)<br>
> -declare float @fmodf(float, float)<br>
> -declare float @logf(float)<br>
> -declare float @log10f(float)<br>
> -declare float @powf(float, float)<br>
> -declare float @sinf(float)<br>
> -declare float @sinhf(float)<br>
> -declare float @sqrtf(float)<br>
> -declare float @tanf(float)<br>
> -declare float @tanhf(float)<br>
> +declare float @acosf(float) readnone nounwind<br>
> +declare float @asinf(float) readnone nounwind<br>
> +declare float @atanf(float) readnone nounwind<br>
> +declare float @atan2f(float, float) readnone nounwind<br>
> +declare float @ceilf(float) readnone nounwind<br>
> +declare float @cosf(float) readnone nounwind<br>
> +declare float @coshf(float) readnone nounwind<br>
> +declare float @expf(float) readnone nounwind<br>
> +declare float @exp2f(float) readnone nounwind<br>
> +declare float @fabsf(float) readnone nounwind<br>
> +declare float @floorf(float) readnone nounwind<br>
> +declare float @fmodf(float, float) readnone nounwind<br>
> +declare float @logf(float) readnone nounwind<br>
> +declare float @log10f(float) readnone nounwind<br>
> +declare float @powf(float, float) readnone nounwind<br>
> +declare float @sinf(float) readnone nounwind<br>
> +declare float @sinhf(float) readnone nounwind<br>
> +declare float @sqrtf(float) readnone nounwind<br>
> +declare float @tanf(float) readnone nounwind<br>
> +declare float @tanhf(float) readnone nounwind<br>
><br>
> define double @T() {<br>
> ; CHECK-LABEL: @T(<br>
><br>
> Modified: llvm/trunk/test/Transforms/InstCombine/call.ll<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/InstCombine/call.ll?rev=276389&r1=276388&r2=276389&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/InstCombine/call.ll?rev=276389&r1=276388&r2=276389&view=diff</a><br>
> ==============================================================================<br>
> --- llvm/trunk/test/Transforms/InstCombine/call.ll (original)<br>
> +++ llvm/trunk/test/Transforms/InstCombine/call.ll Thu Jul 21 23:54:44 2016<br>
> @@ -276,3 +276,14 @@ define <2 x i16> @test16() {<br>
> %X = call <2 x i16> bitcast (i32 ()* @test16a to <2 x i16> ()*)( )<br>
> ret <2 x i16> %X<br>
> }<br>
> +<br>
> +declare i32 @pr28655(i32 returned %V)<br>
> +<br>
> +define i32 @test17() {<br>
> +entry:<br>
> + %C = call i32 @pr28655(i32 0)<br>
> + ret i32 %C<br>
> +}<br>
> +; CHECK-LABEL: @test17(<br>
> +; CHECK: call i32 @pr28655(i32 0)<br>
> +; CHECK: ret i32 0<br>
><br>
> Modified: llvm/trunk/test/Transforms/InstCombine/log-pow.ll<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/InstCombine/log-pow.ll?rev=276389&r1=276388&r2=276389&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/InstCombine/log-pow.ll?rev=276389&r1=276388&r2=276389&view=diff</a><br>
> ==============================================================================<br>
> --- llvm/trunk/test/Transforms/InstCombine/log-pow.ll (original)<br>
> +++ llvm/trunk/test/Transforms/InstCombine/log-pow.ll Thu Jul 21 23:54:44 2016<br>
> @@ -55,7 +55,8 @@ define double @log_exp2_not_fast(double<br>
> ; CHECK-NEXT: %call3 = call fast double @log(double %call2)<br>
> ; CHECK-NEXT: ret double %call3<br>
><br>
> -declare double @log(double)<br>
> +declare double @log(double) #0<br>
> declare double @exp2(double)<br>
> declare double @llvm.pow.f64(double, double)<br>
><br>
> +attributes #0 = { nounwind readnone }<br>
><br>
><br>
> _______________________________________________<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/mailman/listinfo/llvm-commits</a><br>
</div></div></blockquote></div><br></div>