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

llvm-svn: 281160

1 files changed, 20 insertions, 5 deletions

diff --git a/llvm/lib/Transforms/Utils/SimplifyCFG.cpp b/llvm/lib/Transforms/Utils/SimplifyCFG.cpp
index 921f325f29a..64e75548e0f 100644
--- a/llvm/lib/Transforms/Utils/SimplifyCFG.cpp
+++ b/llvm/lib/Transforms/Utils/SimplifyCFG.cpp
@@ -1682,8 +1682,7 @@ static bool SinkThenElseCodeToEnd(BranchInst *BI1) {
     --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(BranchInst *BI1) {
   };
 
   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(BranchInst *BI1) {
 
     // 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;