[llvm] r301950 - SpeculativeExecution: Stop using whitelist for costs
Mikael Holmén via llvm-commits
llvm-commits at lists.llvm.org
Wed May 10 06:31:47 PDT 2017
Hi,
I wrote the TR and contacted Matt about it but I didn't dare to just
revert the commit.
(I also sent an email to llvm-dev when I noticed noone had said anything
about the TR.)
At least I'm happy I'm not the only one noticing this because it looked
really broken to me.
Regards,
Mikael
On 05/10/2017 03:07 PM, Chandler Carruth via llvm-commits wrote:
> It's the same core bug.
>
> In the future, please revert to green when a bug like that comes in?
> Could have saved me a great deal of trouble isolating this.
>
>
> On Wed, May 10, 2017, 05:59 Tom Stellard <tstellar at redhat.com
> <mailto:tstellar at redhat.com>> wrote:
>
> On 05/10/2017 08:40 AM, Chandler Carruth via llvm-commits wrote:
> > FYI, I'm reverting this. See the review thread
> (https://reviews.llvm.org/D24544) where I've provided details on the
> fundamental issue.
> >
>
> There was another bug introduced by this commit too:
>
> https://bugs.llvm.org//show_bug.cgi?id=32964
>
> -Tom
> > On Tue, May 2, 2017 at 11:15 AM Matt Arsenault via llvm-commits
> <llvm-commits at lists.llvm.org <mailto:llvm-commits at lists.llvm.org>
> <mailto:llvm-commits at lists.llvm.org
> <mailto:llvm-commits at lists.llvm.org>>> wrote:
> >
> > Author: arsenm
> > Date: Tue May 2 13:02:18 2017
> > New Revision: 301950
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=301950&view=rev
> > Log:
> > SpeculativeExecution: Stop using whitelist for costs
> >
> > Just let TTI's cost do this instead of arbitrarily restricting
> > this.
> >
> > Added:
> > llvm/trunk/test/Transforms/SpeculativeExecution/spec-other.ll
> > llvm/trunk/test/Transforms/SpeculativeExecution/spec-vector.ll
> > Modified:
> > llvm/trunk/lib/Transforms/Scalar/SpeculativeExecution.cpp
> >
> > Modified:
> llvm/trunk/lib/Transforms/Scalar/SpeculativeExecution.cpp
> > URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/SpeculativeExecution.cpp?rev=301950&r1=301949&r2=301950&view=diff
> >
> ==============================================================================
> > --- llvm/trunk/lib/Transforms/Scalar/SpeculativeExecution.cpp
> (original)
> > +++ llvm/trunk/lib/Transforms/Scalar/SpeculativeExecution.cpp
> Tue May 2 13:02:18 2017
> > @@ -208,47 +208,6 @@ bool SpeculativeExecutionPass::runOnBasi
> > return false;
> > }
> >
> > -static unsigned ComputeSpeculationCost(const Instruction *I,
> > - const
> TargetTransformInfo &TTI) {
> > - switch (Operator::getOpcode(I)) {
> > - case Instruction::GetElementPtr:
> > - case Instruction::Add:
> > - case Instruction::Mul:
> > - case Instruction::And:
> > - case Instruction::Or:
> > - case Instruction::Select:
> > - case Instruction::Shl:
> > - case Instruction::Sub:
> > - case Instruction::LShr:
> > - case Instruction::AShr:
> > - case Instruction::Xor:
> > - case Instruction::ZExt:
> > - case Instruction::SExt:
> > - case Instruction::Call:
> > - case Instruction::BitCast:
> > - case Instruction::PtrToInt:
> > - case Instruction::IntToPtr:
> > - case Instruction::AddrSpaceCast:
> > - case Instruction::FPToUI:
> > - case Instruction::FPToSI:
> > - case Instruction::UIToFP:
> > - case Instruction::SIToFP:
> > - case Instruction::FPExt:
> > - case Instruction::FPTrunc:
> > - case Instruction::FAdd:
> > - case Instruction::FSub:
> > - case Instruction::FMul:
> > - case Instruction::FDiv:
> > - case Instruction::FRem:
> > - case Instruction::ICmp:
> > - case Instruction::FCmp:
> > - return TTI.getUserCost(I);
> > -
> > - default:
> > - return UINT_MAX; // Disallow anything not whitelisted.
> > - }
> > -}
> > -
> > bool SpeculativeExecutionPass::considerHoistingFromTo(
> > BasicBlock &FromBlock, BasicBlock &ToBlock) {
> > SmallSet<const Instruction *, 8> NotHoisted;
> > @@ -264,7 +223,7 @@ bool SpeculativeExecutionPass::considerH
> >
> > unsigned TotalSpeculationCost = 0;
> > for (auto& I : FromBlock) {
> > - const unsigned Cost = ComputeSpeculationCost(&I, *TTI);
> > + const unsigned Cost = TTI->getUserCost(&I);
> > if (Cost != UINT_MAX && isSafeToSpeculativelyExecute(&I) &&
> > AllPrecedingUsesFromBlockHoisted(&I)) {
> > TotalSpeculationCost += Cost;
> >
> > Added:
> llvm/trunk/test/Transforms/SpeculativeExecution/spec-other.ll
> > URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/SpeculativeExecution/spec-other.ll?rev=301950&view=auto
> >
> ==============================================================================
> > ---
> llvm/trunk/test/Transforms/SpeculativeExecution/spec-other.ll (added)
> > +++
> llvm/trunk/test/Transforms/SpeculativeExecution/spec-other.ll Tue
> May 2 13:02:18 2017
> > @@ -0,0 +1,32 @@
> > +; RUN: opt < %s -S -speculative-execution \
> > +; RUN: -spec-exec-max-speculation-cost 4
> -spec-exec-max-not-hoisted 3 \
> > +; RUN: | FileCheck %s
> > +
> > +; CHECK-LABEL: @ifThen_extractvalue(
> > +; CHECK: extractvalue
> > +; CHECK: br i1 true
> > +define void @ifThen_extractvalue() {
> > + br i1 true, label %a, label %b
> > +
> > +a:
> > + %x = extractvalue { i32, i32 } undef, 0
> > + br label %b
> > +
> > +b:
> > + ret void
> > +}
> > +
> > +; CHECK-LABEL: @ifThen_insertvalue(
> > +; CHECK: insertvalue
> > +; CHECK: br i1 true
> > +define void @ifThen_insertvalue() {
> > + br i1 true, label %a, label %b
> > +
> > +a:
> > + %x = insertvalue { i32, i32 } undef, i32 undef, 0
> > + br label %b
> > +
> > +b:
> > + ret void
> > +}
> > +
> >
> > Added:
> llvm/trunk/test/Transforms/SpeculativeExecution/spec-vector.ll
> > URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/SpeculativeExecution/spec-vector.ll?rev=301950&view=auto
> >
> ==============================================================================
> > ---
> llvm/trunk/test/Transforms/SpeculativeExecution/spec-vector.ll (added)
> > +++
> llvm/trunk/test/Transforms/SpeculativeExecution/spec-vector.ll Tue
> May 2 13:02:18 2017
> > @@ -0,0 +1,73 @@
> > +; RUN: opt < %s -S -speculative-execution \
> > +; RUN: -spec-exec-max-speculation-cost 4
> -spec-exec-max-not-hoisted 3 \
> > +; RUN: | FileCheck %s
> > +
> > +; CHECK-LABEL: @ifThen_extractelement_constindex(
> > +; CHECK: extractelement
> > +; CHECK: br i1 true
> > +define void @ifThen_extractelement_constindex() {
> > + br i1 true, label %a, label %b
> > +
> > +a:
> > + %x = extractelement <4 x i32> undef, i32 0
> > + br label %b
> > +
> > +b:
> > + ret void
> > +}
> > +
> > +; CHECK-LABEL: @ifThen_extractelement_varindex(
> > +; CHECK: extractelement
> > +; CHECK: br i1 true
> > +define void @ifThen_extractelement_varindex(i32 %idx) {
> > + br i1 true, label %a, label %b
> > +
> > +a:
> > + %x = extractelement <4 x i32> undef, i32 %idx
> > + br label %b
> > +
> > +b:
> > + ret void
> > +}
> > +
> > +; CHECK-LABEL: @ifThen_insertelement_constindex(
> > +; CHECK: insertelement
> > +; CHECK: br i1 true
> > +define void @ifThen_insertelement_constindex() {
> > + br i1 true, label %a, label %b
> > +
> > +a:
> > + %x = insertelement <4 x i32> undef, i32 undef, i32 0
> > + br label %b
> > +
> > +b:
> > + ret void
> > +}
> > +
> > +; CHECK-LABEL: @ifThen_insertelement_varindex(
> > +; CHECK: insertelement
> > +; CHECK: br i1 true
> > +define void @ifThen_insertelement_varindex(i32 %idx) {
> > + br i1 true, label %a, label %b
> > +
> > +a:
> > + %x = insertelement <4 x i32> undef, i32 undef, i32 %idx
> > + br label %b
> > +
> > +b:
> > + ret void
> > +}
> > +
> > +; CHECK-LABEL: @ifThen_shufflevector(
> > +; CHECK: shufflevector
> > +; CHECK: br i1 true
> > +define void @ifThen_shufflevector() {
> > + br i1 true, label %a, label %b
> > +
> > +a:
> > + %x = shufflevector <4 x i32> undef, <4 x i32> undef, <4 x
> i32> undef
> > + br label %b
> > +
> > +b:
> > + ret void
> > +}
> >
> >
> > _______________________________________________
> > llvm-commits mailing list
> > llvm-commits at lists.llvm.org
> <mailto:llvm-commits at lists.llvm.org>
> <mailto:llvm-commits at lists.llvm.org
> <mailto:llvm-commits at lists.llvm.org>>
> > http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
> >
> >
> >
> > _______________________________________________
> > llvm-commits mailing list
> > llvm-commits at lists.llvm.org <mailto:llvm-commits at lists.llvm.org>
> > http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
> >
>
>
>
> _______________________________________________
> 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