<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>