[llvm] r281160 - [SimplifyCFG] Harden up the profitability heuristic for block splitting during sinking

[James Molloy via llvm-commits]

Author: jamesm
Date: Sun Sep 11 03:07:30 2016
New Revision: 281160

URL: http://llvm.org/viewvc/llvm-project?rev=281160&view=rev
Log:
[SimplifyCFG] Harden up the profitability heuristic for block splitting during sinking

Exposed by PR30244, we will split a block currently if we think we can sink at least one instruction. However this isn't right - the reason we split predecessors is so that we can sink instructions that otherwise couldn't be sunk because it isn't safe to do so - stores, for example.

So, change the heuristic to only split if it thinks it can sink at least one non-speculatable instruction.

Should fix PR30244.

Modified:
    llvm/trunk/lib/Transforms/Utils/SimplifyCFG.cpp
    llvm/trunk/test/Transforms/SimplifyCFG/sink-common-code.ll

Modified: llvm/trunk/lib/Transforms/Utils/SimplifyCFG.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Utils/SimplifyCFG.cpp?rev=281160&r1=281159&r2=281160&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/Utils/SimplifyCFG.cpp (original)
+++ llvm/trunk/lib/Transforms/Utils/SimplifyCFG.cpp Sun Sep 11 03:07:30 2016
@@ -1682,8 +1682,7 @@ static bool SinkThenElseCodeToEnd(Branch
     --LRI;
   }
 
-  auto ProfitableToSinkLastInstruction = [&]() {
-    LRI.reset();
+  auto ProfitableToSinkInstruction = [&](LockstepReverseIterator &LRI) {
     unsigned NumPHIdValues = 0;
     for (auto *I : *LRI)
       for (auto *V : PHIOperands[I])
@@ -1698,8 +1697,23 @@ static bool SinkThenElseCodeToEnd(Branch
   };
 
   if (ScanIdx > 0 && Cond) {
-    // Check if we would actually sink anything first!
-    if (!ProfitableToSinkLastInstruction())
+    // Check if we would actually sink anything first! This mutates the CFG and
+    // adds an extra block. The goal in doing this is to allow instructions that
+    // couldn't be sunk before to be sunk - obviously, speculatable instructions
+    // (such as trunc, add) can be sunk and predicated already. So we check that
+    // we're going to sink at least one non-speculatable instruction.
+    LRI.reset();
+    unsigned Idx = 0;
+    bool Profitable = false;
+    while (ProfitableToSinkInstruction(LRI) && Idx < ScanIdx) {
+      if (!isSafeToSpeculativelyExecute((*LRI)[0])) {
+        Profitable = true;
+        break;
+      }
+      --LRI;
+      ++Idx;
+    }
+    if (!Profitable)
       return false;
     
     DEBUG(dbgs() << "SINK: Splitting edge\n");
@@ -1731,7 +1745,8 @@ static bool SinkThenElseCodeToEnd(Branch
 
     // Because we've sunk every instruction in turn, the current instruction to
     // sink is always at index 0.
-    if (!ProfitableToSinkLastInstruction()) {
+    LRI.reset();
+    if (!ProfitableToSinkInstruction(LRI)) {
       // Too many PHIs would be created.
       DEBUG(dbgs() << "SINK: stopping here, too many PHIs would be created!\n");
       break;

Modified: llvm/trunk/test/Transforms/SimplifyCFG/sink-common-code.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/SimplifyCFG/sink-common-code.ll?rev=281160&r1=281159&r2=281160&view=diff
==============================================================================
--- llvm/trunk/test/Transforms/SimplifyCFG/sink-common-code.ll (original)
+++ llvm/trunk/test/Transforms/SimplifyCFG/sink-common-code.ll Sun Sep 11 03:07:30 2016
@@ -478,6 +478,35 @@ if.end:
 
 ; CHECK-LABEL: test16
 ; CHECK: zext
+; CHECK: zext
+
+define zeroext i1 @test16a(i1 zeroext %flag, i1 zeroext %flag2, i32 %blksA, i32 %blksB, i32 %nblks, i8* %p) {
+
+entry:
+  br i1 %flag, label %if.then, label %if.else
+
+if.then:
+  %cmp = icmp uge i32 %blksA, %nblks
+  %frombool1 = zext i1 %cmp to i8
+  store i8 %frombool1, i8* %p
+  br label %if.end
+
+if.else:
+  br i1 %flag2, label %if.then2, label %if.end
+
+if.then2:
+  %add = add i32 %nblks, %blksB
+  %cmp2 = icmp ule i32 %add, %blksA
+  %frombool3 = zext i1 %cmp2 to i8
+  store i8 %frombool3, i8* %p
+  br label %if.end
+
+if.end:
+  ret i1 true
+}
+
+; CHECK-LABEL: test16a
+; CHECK: zext
 ; CHECK-NOT: zext
 
 define zeroext i1 @test17(i32 %flag, i32 %blksA, i32 %blksB, i32 %nblks) {
@@ -489,13 +518,13 @@ entry:
 
 if.then:
   %cmp = icmp uge i32 %blksA, %nblks
-  %frombool1 = zext i1 %cmp to i8
+  %frombool1 = call i8 @i1toi8(i1 %cmp)
   br label %if.end
 
 if.then2:
   %add = add i32 %nblks, %blksB
   %cmp2 = icmp ule i32 %add, %blksA
-  %frombool3 = zext i1 %cmp2 to i8
+  %frombool3 = call i8 @i1toi8(i1 %cmp2)
   br label %if.end
 
 if.end:
@@ -503,6 +532,7 @@ if.end:
   %tobool4 = icmp ne i8 %obeys.0, 0
   ret i1 %tobool4
 }
+declare i8 @i1toi8(i1)
 
 ; CHECK-LABEL: test17
 ; CHECK: if.then:
@@ -516,7 +546,7 @@ if.end:
 
 ; CHECK: [[x]]:
 ; CHECK-NEXT: %[[y:.*]] = phi i1 [ %cmp
-; CHECK-NEXT: %[[z:.*]] = zext i1 %[[y]]
+; CHECK-NEXT: %[[z:.*]] = call i8 @i1toi8(i1 %[[y]])
 ; CHECK-NEXT: br label %if.end
 
 ; CHECK: if.end:
@@ -639,6 +669,25 @@ declare void @g()
 ; CHECK-LABEL: test_pr30292
 ; CHECK: phi i32 [ 0, %entry ], [ %add1, %succ ], [ %add2, %two ]
 
+define void @test_pr30244(i1 %cond, i1 %cond2, i32 %a, i32 %b) {
+entry:
+  %add1 = add i32 %a, 1
+  br label %succ
+
+one:
+  br i1 %cond, label %two, label %succ
+
+two:
+  call void @g()
+  %add2 = add i32 %a, 1
+  br label %succ
+
+succ:
+  %p = phi i32 [ 0, %entry ], [ %add1, %one ], [ %add2, %two ]
+  br label %one
+}
+
+
 ; CHECK: !0 = !{!1, !1, i64 0}
 ; CHECK: !1 = !{!"float", !2}
 ; CHECK: !2 = !{!"an example type tree"}