<div dir="ltr">This change does not appear to respect the comments here:<div><div> // If AlternativeV is not nullptr, we care about both incoming values in PHI.</div><div> // PHI must be exactly: phi <ty> [ %BB, %V ], [ %OtherBB, %AlternativeV]</div><div> // where OtherBB is the single other predecessor of BB's only successor.</div></div><div><br></div><div>I suspect this change broke some Chromium tests. I'm in the process of reproducing the failure and testing if a revert fixes things.</div></div><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Dec 14, 2015 at 2:57 AM, James Molloy via llvm-commits <span dir="ltr"><<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: jamesm<br>
Date: Mon Dec 14 04:57:01 2015<br>
New Revision: 255489<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=255489&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=255489&view=rev</a><br>
Log:<br>
Don't create unnecessary PHIs<br>
<br>
In conditional store merging, we were creating PHIs when we didn't<br>
need to. If the value to be predicated isn't defined in the block<br>
we're predicating, then it doesn't need a PHI at all (because we only<br>
deal with triangles and diamonds, any value not in the predicated BB<br>
must dominate the predicated BB).<br>
<br>
This fixes a large code size increase in some benchmarks in a popular embedded benchmark suite.<br>
<br>
Added:<br>
llvm/trunk/test/Transforms/SimplifyCFG/merge-cond-stores-2.ll<br>
Modified:<br>
llvm/trunk/lib/Transforms/Utils/SimplifyCFG.cpp<br>
llvm/trunk/test/Transforms/SimplifyCFG/merge-cond-stores.ll<br>
<br>
Modified: llvm/trunk/lib/Transforms/Utils/SimplifyCFG.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Utils/SimplifyCFG.cpp?rev=255489&r1=255488&r2=255489&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Utils/SimplifyCFG.cpp?rev=255489&r1=255488&r2=255489&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/lib/Transforms/Utils/SimplifyCFG.cpp (original)<br>
+++ llvm/trunk/lib/Transforms/Utils/SimplifyCFG.cpp Mon Dec 14 04:57:01 2015<br>
@@ -2385,6 +2385,11 @@ static Value *ensureValueAvailableInSucc<br>
// If AlternativeV is not nullptr, we care about both incoming values in PHI.<br>
// PHI must be exactly: phi <ty> [ %BB, %V ], [ %OtherBB, %AlternativeV]<br>
// where OtherBB is the single other predecessor of BB's only successor.<br>
+<br>
+ // If V is not an instruction defined in BB, just return it.<br>
+ if (!isa<Instruction>(V) || cast<Instruction>(V)->getParent() != BB)<br>
+ return V;<br>
+<br>
PHINode *PHI = nullptr;<br>
BasicBlock *Succ = BB->getSingleSuccessor();<br>
<br>
<br>
Added: llvm/trunk/test/Transforms/SimplifyCFG/merge-cond-stores-2.ll<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/SimplifyCFG/merge-cond-stores-2.ll?rev=255489&view=auto" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/SimplifyCFG/merge-cond-stores-2.ll?rev=255489&view=auto</a><br>
==============================================================================<br>
--- llvm/trunk/test/Transforms/SimplifyCFG/merge-cond-stores-2.ll (added)<br>
+++ llvm/trunk/test/Transforms/SimplifyCFG/merge-cond-stores-2.ll Mon Dec 14 04:57:01 2015<br>
@@ -0,0 +1,198 @@<br>
+; RUN: opt -S < %s -simplifycfg -simplifycfg-merge-cond-stores=true -simplifycfg-merge-cond-stores-aggressively=false -phi-node-folding-threshold=2 | FileCheck %s<br>
+<br>
+target datalayout = "e-m:e-p:32:32-i64:64-v128:64:128-a:0:32-n32-S64"<br>
+target triple = "armv7--linux-gnueabihf"<br>
+<br>
+; This is a bit reversal that has been run through the early optimizer (-mem2reg -gvn -instcombine).<br>
+; There should be no additional PHIs created at all. The store should be on its own in a predicated<br>
+; block and there should be no PHIs.<br>
+<br>
+; CHECK-LABEL: @f<br>
+; CHECK-NOT: phi<br>
+; CHECK: br i1 {{.*}}, label %[[L:.*]], label %[[R:.*]]<br>
+; CHECK: [[L]] ; preds =<br>
+; CHECK-NEXT: store<br>
+; CHECK-NEXT: br label %[[R]]<br>
+; CHECK: [[R]] ; preds =<br>
+; CHECK-NEXT: ret i32 0<br>
+<br>
+define i32 @f(i32* %b) {<br>
+entry:<br>
+ %0 = load i32, i32* %b, align 4<br>
+ %and = and i32 %0, 1<br>
+ %tobool = icmp eq i32 %and, 0<br>
+ br i1 %tobool, label %if.end, label %if.then<br>
+<br>
+if.then: ; preds = %entry<br>
+ %or = or i32 %0, -2147483648<br>
+ store i32 %or, i32* %b, align 4<br>
+ br label %if.end<br>
+<br>
+if.end: ; preds = %entry, %if.then<br>
+ %1 = phi i32 [ %0, %entry ], [ %or, %if.then ]<br>
+ %and1 = and i32 %1, 2<br>
+ %tobool2 = icmp eq i32 %and1, 0<br>
+ br i1 %tobool2, label %if.end5, label %if.then3<br>
+<br>
+if.then3: ; preds = %if.end<br>
+ %or4 = or i32 %1, 1073741824<br>
+ store i32 %or4, i32* %b, align 4<br>
+ br label %if.end5<br>
+<br>
+if.end5: ; preds = %if.end, %if.then3<br>
+ %2 = phi i32 [ %1, %if.end ], [ %or4, %if.then3 ]<br>
+ %and6 = and i32 %2, 4<br>
+ %tobool7 = icmp eq i32 %and6, 0<br>
+ br i1 %tobool7, label %if.end10, label %if.then8<br>
+<br>
+if.then8: ; preds = %if.end5<br>
+ %or9 = or i32 %2, 536870912<br>
+ store i32 %or9, i32* %b, align 4<br>
+ br label %if.end10<br>
+<br>
+if.end10: ; preds = %if.end5, %if.then8<br>
+ %3 = phi i32 [ %2, %if.end5 ], [ %or9, %if.then8 ]<br>
+ %and11 = and i32 %3, 8<br>
+ %tobool12 = icmp eq i32 %and11, 0<br>
+ br i1 %tobool12, label %if.end15, label %if.then13<br>
+<br>
+if.then13: ; preds = %if.end10<br>
+ %or14 = or i32 %3, 268435456<br>
+ store i32 %or14, i32* %b, align 4<br>
+ br label %if.end15<br>
+<br>
+if.end15: ; preds = %if.end10, %if.then13<br>
+ %4 = phi i32 [ %3, %if.end10 ], [ %or14, %if.then13 ]<br>
+ %and16 = and i32 %4, 16<br>
+ %tobool17 = icmp eq i32 %and16, 0<br>
+ br i1 %tobool17, label %if.end20, label %if.then18<br>
+<br>
+if.then18: ; preds = %if.end15<br>
+ %or19 = or i32 %4, 134217728<br>
+ store i32 %or19, i32* %b, align 4<br>
+ br label %if.end20<br>
+<br>
+if.end20: ; preds = %if.end15, %if.then18<br>
+ %5 = phi i32 [ %4, %if.end15 ], [ %or19, %if.then18 ]<br>
+ %and21 = and i32 %5, 32<br>
+ %tobool22 = icmp eq i32 %and21, 0<br>
+ br i1 %tobool22, label %if.end25, label %if.then23<br>
+<br>
+if.then23: ; preds = %if.end20<br>
+ %or24 = or i32 %5, 67108864<br>
+ store i32 %or24, i32* %b, align 4<br>
+ br label %if.end25<br>
+<br>
+if.end25: ; preds = %if.end20, %if.then23<br>
+ %6 = phi i32 [ %5, %if.end20 ], [ %or24, %if.then23 ]<br>
+ %and26 = and i32 %6, 64<br>
+ %tobool27 = icmp eq i32 %and26, 0<br>
+ br i1 %tobool27, label %if.end30, label %if.then28<br>
+<br>
+if.then28: ; preds = %if.end25<br>
+ %or29 = or i32 %6, 33554432<br>
+ store i32 %or29, i32* %b, align 4<br>
+ br label %if.end30<br>
+<br>
+if.end30: ; preds = %if.end25, %if.then28<br>
+ %7 = phi i32 [ %6, %if.end25 ], [ %or29, %if.then28 ]<br>
+ %and31 = and i32 %7, 256<br>
+ %tobool32 = icmp eq i32 %and31, 0<br>
+ br i1 %tobool32, label %if.end35, label %if.then33<br>
+<br>
+if.then33: ; preds = %if.end30<br>
+ %or34 = or i32 %7, 8388608<br>
+ store i32 %or34, i32* %b, align 4<br>
+ br label %if.end35<br>
+<br>
+if.end35: ; preds = %if.end30, %if.then33<br>
+ %8 = phi i32 [ %7, %if.end30 ], [ %or34, %if.then33 ]<br>
+ %and36 = and i32 %8, 512<br>
+ %tobool37 = icmp eq i32 %and36, 0<br>
+ br i1 %tobool37, label %if.end40, label %if.then38<br>
+<br>
+if.then38: ; preds = %if.end35<br>
+ %or39 = or i32 %8, 4194304<br>
+ store i32 %or39, i32* %b, align 4<br>
+ br label %if.end40<br>
+<br>
+if.end40: ; preds = %if.end35, %if.then38<br>
+ %9 = phi i32 [ %8, %if.end35 ], [ %or39, %if.then38 ]<br>
+ %and41 = and i32 %9, 1024<br>
+ %tobool42 = icmp eq i32 %and41, 0<br>
+ br i1 %tobool42, label %if.end45, label %if.then43<br>
+<br>
+if.then43: ; preds = %if.end40<br>
+ %or44 = or i32 %9, 2097152<br>
+ store i32 %or44, i32* %b, align 4<br>
+ br label %if.end45<br>
+<br>
+if.end45: ; preds = %if.end40, %if.then43<br>
+ %10 = phi i32 [ %9, %if.end40 ], [ %or44, %if.then43 ]<br>
+ %and46 = and i32 %10, 2048<br>
+ %tobool47 = icmp eq i32 %and46, 0<br>
+ br i1 %tobool47, label %if.end50, label %if.then48<br>
+<br>
+if.then48: ; preds = %if.end45<br>
+ %or49 = or i32 %10, 1048576<br>
+ store i32 %or49, i32* %b, align 4<br>
+ br label %if.end50<br>
+<br>
+if.end50: ; preds = %if.end45, %if.then48<br>
+ %11 = phi i32 [ %10, %if.end45 ], [ %or49, %if.then48 ]<br>
+ %and51 = and i32 %11, 4096<br>
+ %tobool52 = icmp eq i32 %and51, 0<br>
+ br i1 %tobool52, label %if.end55, label %if.then53<br>
+<br>
+if.then53: ; preds = %if.end50<br>
+ %or54 = or i32 %11, 524288<br>
+ store i32 %or54, i32* %b, align 4<br>
+ br label %if.end55<br>
+<br>
+if.end55: ; preds = %if.end50, %if.then53<br>
+ %12 = phi i32 [ %11, %if.end50 ], [ %or54, %if.then53 ]<br>
+ %and56 = and i32 %12, 8192<br>
+ %tobool57 = icmp eq i32 %and56, 0<br>
+ br i1 %tobool57, label %if.end60, label %if.then58<br>
+<br>
+if.then58: ; preds = %if.end55<br>
+ %or59 = or i32 %12, 262144<br>
+ store i32 %or59, i32* %b, align 4<br>
+ br label %if.end60<br>
+<br>
+if.end60: ; preds = %if.end55, %if.then58<br>
+ %13 = phi i32 [ %12, %if.end55 ], [ %or59, %if.then58 ]<br>
+ %and61 = and i32 %13, 16384<br>
+ %tobool62 = icmp eq i32 %and61, 0<br>
+ br i1 %tobool62, label %if.end65, label %if.then63<br>
+<br>
+if.then63: ; preds = %if.end60<br>
+ %or64 = or i32 %13, 131072<br>
+ store i32 %or64, i32* %b, align 4<br>
+ br label %if.end65<br>
+<br>
+if.end65: ; preds = %if.end60, %if.then63<br>
+ %14 = phi i32 [ %13, %if.end60 ], [ %or64, %if.then63 ]<br>
+ %and66 = and i32 %14, 32768<br>
+ %tobool67 = icmp eq i32 %and66, 0<br>
+ br i1 %tobool67, label %if.end70, label %if.then68<br>
+<br>
+if.then68: ; preds = %if.end65<br>
+ %or69 = or i32 %14, 65536<br>
+ store i32 %or69, i32* %b, align 4<br>
+ br label %if.end70<br>
+<br>
+if.end70: ; preds = %if.end65, %if.then68<br>
+ %15 = phi i32 [ %14, %if.end65 ], [ %or69, %if.then68 ]<br>
+ %and71 = and i32 %15, 128<br>
+ %tobool72 = icmp eq i32 %and71, 0<br>
+ br i1 %tobool72, label %if.end75, label %if.then73<br>
+<br>
+if.then73: ; preds = %if.end70<br>
+ %or74 = or i32 %15, 16777216<br>
+ store i32 %or74, i32* %b, align 4<br>
+ br label %if.end75<br>
+<br>
+if.end75: ; preds = %if.end70, %if.then73<br>
+ ret i32 0<br>
+}<br>
<br>
Modified: llvm/trunk/test/Transforms/SimplifyCFG/merge-cond-stores.ll<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/SimplifyCFG/merge-cond-stores.ll?rev=255489&r1=255488&r2=255489&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/SimplifyCFG/merge-cond-stores.ll?rev=255489&r1=255488&r2=255489&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/test/Transforms/SimplifyCFG/merge-cond-stores.ll (original)<br>
+++ llvm/trunk/test/Transforms/SimplifyCFG/merge-cond-stores.ll Mon Dec 14 04:57:01 2015<br>
@@ -2,10 +2,8 @@<br>
<br>
; CHECK-LABEL: @test_simple<br>
; This test should succeed and end up if-converted.<br>
-; CHECK: icmp eq i32 %b, 0<br>
-; CHECK-NEXT: icmp ne i32 %a, 0<br>
-; CHECK-NEXT: xor i1 %x2, true<br>
-; CHECK-NEXT: %[[x:.*]] = or i1 %{{.*}}, %{{.*}}<br>
+; CHECK: %[[A:.*]] = or i32 %a, %b<br>
+; CHECK-NEXT: %[[x:.*]] = icmp eq i32 %[[A]], 0<br>
; CHECK-NEXT: br i1 %[[x]]<br>
; CHECK: store<br>
; CHECK-NOT: store<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>
</blockquote></div><br></div>