[llvm] r276389 - Don't remove side effecting instructions due to ConstantFoldInstruction

Hans Wennborg via llvm-commits llvm-commits at lists.llvm.org
Mon Jul 25 10:35:18 PDT 2016


Thanks! Merged in r276660.

 - Hans

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


More information about the llvm-commits mailing list