<p dir="ltr">It's the same core bug.</p>
<p dir="ltr">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.</p>
<br><div class="gmail_quote"><div dir="ltr">On Wed, May 10, 2017, 05:59 Tom Stellard <<a href="mailto:tstellar@redhat.com">tstellar@redhat.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">On 05/10/2017 08:40 AM, Chandler Carruth via llvm-commits wrote:<br>
> FYI, I'm reverting this. See the review thread (<a href="https://reviews.llvm.org/D24544" rel="noreferrer" target="_blank">https://reviews.llvm.org/D24544</a>) where I've provided details on the fundamental issue.<br>
><br>
<br>
There was another bug introduced by this commit too:<br>
<br>
<a href="https://bugs.llvm.org//show_bug.cgi?id=32964" rel="noreferrer" target="_blank">https://bugs.llvm.org//show_bug.cgi?id=32964</a><br>
<br>
-Tom<br>
> On Tue, May 2, 2017 at 11:15 AM Matt Arsenault via llvm-commits <<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a> <mailto:<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a>>> wrote:<br>
><br>
> Author: arsenm<br>
> Date: Tue May 2 13:02:18 2017<br>
> New Revision: 301950<br>
><br>
> URL: <a href="http://llvm.org/viewvc/llvm-project?rev=301950&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=301950&view=rev</a><br>
> Log:<br>
> SpeculativeExecution: Stop using whitelist for costs<br>
><br>
> Just let TTI's cost do this instead of arbitrarily restricting<br>
> this.<br>
><br>
> Added:<br>
> llvm/trunk/test/Transforms/SpeculativeExecution/spec-other.ll<br>
> llvm/trunk/test/Transforms/SpeculativeExecution/spec-vector.ll<br>
> Modified:<br>
> llvm/trunk/lib/Transforms/Scalar/SpeculativeExecution.cpp<br>
><br>
> Modified: llvm/trunk/lib/Transforms/Scalar/SpeculativeExecution.cpp<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/SpeculativeExecution.cpp?rev=301950&r1=301949&r2=301950&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/SpeculativeExecution.cpp?rev=301950&r1=301949&r2=301950&view=diff</a><br>
> ==============================================================================<br>
> --- llvm/trunk/lib/Transforms/Scalar/SpeculativeExecution.cpp (original)<br>
> +++ llvm/trunk/lib/Transforms/Scalar/SpeculativeExecution.cpp Tue May 2 13:02:18 2017<br>
> @@ -208,47 +208,6 @@ bool SpeculativeExecutionPass::runOnBasi<br>
> return false;<br>
> }<br>
><br>
> -static unsigned ComputeSpeculationCost(const Instruction *I,<br>
> - const TargetTransformInfo &TTI) {<br>
> - switch (Operator::getOpcode(I)) {<br>
> - case Instruction::GetElementPtr:<br>
> - case Instruction::Add:<br>
> - case Instruction::Mul:<br>
> - case Instruction::And:<br>
> - case Instruction::Or:<br>
> - case Instruction::Select:<br>
> - case Instruction::Shl:<br>
> - case Instruction::Sub:<br>
> - case Instruction::LShr:<br>
> - case Instruction::AShr:<br>
> - case Instruction::Xor:<br>
> - case Instruction::ZExt:<br>
> - case Instruction::SExt:<br>
> - case Instruction::Call:<br>
> - case Instruction::BitCast:<br>
> - case Instruction::PtrToInt:<br>
> - case Instruction::IntToPtr:<br>
> - case Instruction::AddrSpaceCast:<br>
> - case Instruction::FPToUI:<br>
> - case Instruction::FPToSI:<br>
> - case Instruction::UIToFP:<br>
> - case Instruction::SIToFP:<br>
> - case Instruction::FPExt:<br>
> - case Instruction::FPTrunc:<br>
> - case Instruction::FAdd:<br>
> - case Instruction::FSub:<br>
> - case Instruction::FMul:<br>
> - case Instruction::FDiv:<br>
> - case Instruction::FRem:<br>
> - case Instruction::ICmp:<br>
> - case Instruction::FCmp:<br>
> - return TTI.getUserCost(I);<br>
> -<br>
> - default:<br>
> - return UINT_MAX; // Disallow anything not whitelisted.<br>
> - }<br>
> -}<br>
> -<br>
> bool SpeculativeExecutionPass::considerHoistingFromTo(<br>
> BasicBlock &FromBlock, BasicBlock &ToBlock) {<br>
> SmallSet<const Instruction *, 8> NotHoisted;<br>
> @@ -264,7 +223,7 @@ bool SpeculativeExecutionPass::considerH<br>
><br>
> unsigned TotalSpeculationCost = 0;<br>
> for (auto& I : FromBlock) {<br>
> - const unsigned Cost = ComputeSpeculationCost(&I, *TTI);<br>
> + const unsigned Cost = TTI->getUserCost(&I);<br>
> if (Cost != UINT_MAX && isSafeToSpeculativelyExecute(&I) &&<br>
> AllPrecedingUsesFromBlockHoisted(&I)) {<br>
> TotalSpeculationCost += Cost;<br>
><br>
> Added: llvm/trunk/test/Transforms/SpeculativeExecution/spec-other.ll<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/SpeculativeExecution/spec-other.ll?rev=301950&view=auto" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/SpeculativeExecution/spec-other.ll?rev=301950&view=auto</a><br>
> ==============================================================================<br>
> --- llvm/trunk/test/Transforms/SpeculativeExecution/spec-other.ll (added)<br>
> +++ llvm/trunk/test/Transforms/SpeculativeExecution/spec-other.ll Tue May 2 13:02:18 2017<br>
> @@ -0,0 +1,32 @@<br>
> +; RUN: opt < %s -S -speculative-execution \<br>
> +; RUN: -spec-exec-max-speculation-cost 4 -spec-exec-max-not-hoisted 3 \<br>
> +; RUN: | FileCheck %s<br>
> +<br>
> +; CHECK-LABEL: @ifThen_extractvalue(<br>
> +; CHECK: extractvalue<br>
> +; CHECK: br i1 true<br>
> +define void @ifThen_extractvalue() {<br>
> + br i1 true, label %a, label %b<br>
> +<br>
> +a:<br>
> + %x = extractvalue { i32, i32 } undef, 0<br>
> + br label %b<br>
> +<br>
> +b:<br>
> + ret void<br>
> +}<br>
> +<br>
> +; CHECK-LABEL: @ifThen_insertvalue(<br>
> +; CHECK: insertvalue<br>
> +; CHECK: br i1 true<br>
> +define void @ifThen_insertvalue() {<br>
> + br i1 true, label %a, label %b<br>
> +<br>
> +a:<br>
> + %x = insertvalue { i32, i32 } undef, i32 undef, 0<br>
> + br label %b<br>
> +<br>
> +b:<br>
> + ret void<br>
> +}<br>
> +<br>
><br>
> Added: llvm/trunk/test/Transforms/SpeculativeExecution/spec-vector.ll<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/SpeculativeExecution/spec-vector.ll?rev=301950&view=auto" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/SpeculativeExecution/spec-vector.ll?rev=301950&view=auto</a><br>
> ==============================================================================<br>
> --- llvm/trunk/test/Transforms/SpeculativeExecution/spec-vector.ll (added)<br>
> +++ llvm/trunk/test/Transforms/SpeculativeExecution/spec-vector.ll Tue May 2 13:02:18 2017<br>
> @@ -0,0 +1,73 @@<br>
> +; RUN: opt < %s -S -speculative-execution \<br>
> +; RUN: -spec-exec-max-speculation-cost 4 -spec-exec-max-not-hoisted 3 \<br>
> +; RUN: | FileCheck %s<br>
> +<br>
> +; CHECK-LABEL: @ifThen_extractelement_constindex(<br>
> +; CHECK: extractelement<br>
> +; CHECK: br i1 true<br>
> +define void @ifThen_extractelement_constindex() {<br>
> + br i1 true, label %a, label %b<br>
> +<br>
> +a:<br>
> + %x = extractelement <4 x i32> undef, i32 0<br>
> + br label %b<br>
> +<br>
> +b:<br>
> + ret void<br>
> +}<br>
> +<br>
> +; CHECK-LABEL: @ifThen_extractelement_varindex(<br>
> +; CHECK: extractelement<br>
> +; CHECK: br i1 true<br>
> +define void @ifThen_extractelement_varindex(i32 %idx) {<br>
> + br i1 true, label %a, label %b<br>
> +<br>
> +a:<br>
> + %x = extractelement <4 x i32> undef, i32 %idx<br>
> + br label %b<br>
> +<br>
> +b:<br>
> + ret void<br>
> +}<br>
> +<br>
> +; CHECK-LABEL: @ifThen_insertelement_constindex(<br>
> +; CHECK: insertelement<br>
> +; CHECK: br i1 true<br>
> +define void @ifThen_insertelement_constindex() {<br>
> + br i1 true, label %a, label %b<br>
> +<br>
> +a:<br>
> + %x = insertelement <4 x i32> undef, i32 undef, i32 0<br>
> + br label %b<br>
> +<br>
> +b:<br>
> + ret void<br>
> +}<br>
> +<br>
> +; CHECK-LABEL: @ifThen_insertelement_varindex(<br>
> +; CHECK: insertelement<br>
> +; CHECK: br i1 true<br>
> +define void @ifThen_insertelement_varindex(i32 %idx) {<br>
> + br i1 true, label %a, label %b<br>
> +<br>
> +a:<br>
> + %x = insertelement <4 x i32> undef, i32 undef, i32 %idx<br>
> + br label %b<br>
> +<br>
> +b:<br>
> + ret void<br>
> +}<br>
> +<br>
> +; CHECK-LABEL: @ifThen_shufflevector(<br>
> +; CHECK: shufflevector<br>
> +; CHECK: br i1 true<br>
> +define void @ifThen_shufflevector() {<br>
> + br i1 true, label %a, label %b<br>
> +<br>
> +a:<br>
> + %x = shufflevector <4 x i32> undef, <4 x i32> undef, <4 x i32> undef<br>
> + br label %b<br>
> +<br>
> +b:<br>
> + ret void<br>
> +}<br>
><br>
><br>
> _______________________________________________<br>
> llvm-commits mailing list<br>
> <a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a> <mailto:<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">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>
><br>
><br>
><br>
> _______________________________________________<br>
> llvm-commits mailing list<br>
> <a href="mailto:llvm-commits@lists.llvm.org" target="_blank">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>
><br>
<br>
</blockquote></div>