[simplify-cfg] When moving cond_fail to preds, do not bail early.

include/swift/SILOptimizer/PassManager/Passes.def

@@ -169,6 +169,9 @@ PASS(LSLocationPrinter, "lslocation-dump",
      "Dump LSLocation results from analyzing all accessed locations")
 PASS(MergeCondFails, "merge-cond_fails",
      "Remove redundant overflow checks")
+PASS(MoveCondFailToPreds, "move-cond-fail-to-preds",
+     "Test pass that hoists conditional fails to predecessors blocks when "
+     "profitable")
 PASS(NoReturnFolding, "noreturn-folding",
      "Add 'unreachable' after noreturn calls")
 PASS(RCIdentityDumper, "rc-id-dumper",

lib/SILOptimizer/Transforms/SimplifyCFG.cpp

@@ -2121,18 +2121,22 @@ static bool tryMoveCondFailToPreds(SILBasicBlock *BB) {
     // execute the cond_fail speculatively.
     if (!Pred->getSingleSuccessor())
       return false;
-
+
+    // If we already found a constant pred, we do not need to check the incoming
+    // value to see if it is constant. We are already going to perform the
+    // optimization.
+    if (somePredsAreConst)
+      continue;
+
     SILValue incoming = condArg->getIncomingValue(Pred);
-    if (isa<IntegerLiteralInst>(incoming)) {
-      somePredsAreConst = true;
-      break;
-    }
+    somePredsAreConst |= isa<IntegerLiteralInst>(incoming);
   }
+
   if (!somePredsAreConst)
     return false;
-  
+
   DEBUG(llvm::dbgs() << "move to predecessors: " << *CFI);
-  
+
   // Move the cond_fail to the predecessor blocks.
   for (auto *Pred : BB->getPreds()) {
     SILValue incoming = condArg->getIncomingValue(Pred);
@@ -3504,6 +3508,22 @@ class SROABBArgs : public SILFunctionTransform {
   StringRef getName() override { return "SROA BB Arguments"; }
 };
 
+// Used to test tryMoveCondFailToPreds with sil-opt
+class MoveCondFailToPreds : public SILFunctionTransform {
+public:
+  MoveCondFailToPreds() {}
+  void run() override {
+    for (auto &BB : *getFunction()) {
+      if (tryMoveCondFailToPreds(&BB)) {
+        invalidateAnalysis(
+            SILAnalysis::InvalidationKind::BranchesAndInstructions);
+      }
+    }
+  }
+
+  StringRef getName() override { return "Move Cond Fail To Preds"; }
+};
+
 } // End anonymous namespace.
 
 /// Splits all critical edges in a function.
@@ -3523,3 +3543,8 @@ SILTransform *swift::createSROABBArgs() { return new SROABBArgs(); }
 SILTransform *swift::createSimplifyBBArgs() {
   return new SimplifyBBArgs();
 }
+
+// Moves cond_fail instructions to predecessors.
+SILTransform *swift::createMoveCondFailToPreds() {
+  return new MoveCondFailToPreds();
+}

test/SILOptimizer/move_cond_fail_simplify_cfg.sil

@@ -0,0 +1,45 @@
+// RUN: %target-sil-opt -enable-sil-verify-all %s -move-cond-fail-to-preds | FileCheck %s
+
+import Builtin
+import Swift
+
+sil_stage canonical
+
+// Make sure that we actually check all predecessors when we try to hoist
+// cond_fails.
+//
+// CHECK-LABEL: sil @check_all_preds_when_hoisting_cond_fails : $@convention(thin) (Builtin.Int1) -> () {
+sil @check_all_preds_when_hoisting_cond_fails : $@convention(thin) (Builtin.Int1) -> () {
+bb0(%0 : $Builtin.Int1):
+  %1 = integer_literal $Builtin.Int1, 0
+  %2 = integer_literal $Builtin.Int64, 1
+  %3 = integer_literal $Builtin.Int64, 3
+  cond_br %0, bb3, bb2
+
+// Make sure that the predecessor order here is not changed. This is necessary
+// for the test to actually test that we are checking /all/ predecessors.
+//
+// CHECK: bb1({{.*}}): // Preds: bb2 bb1
+bb1(%7 : $Builtin.Int64, %8 : $Builtin.Int64, %9 : $Builtin.Int1):
+  %10 = builtin "sadd_with_overflow_Int64"(%8 : $Builtin.Int64, %2 : $Builtin.Int64, %1 : $Builtin.Int1) : $(Builtin.Int64, Builtin.Int1)
+  %11 = tuple_extract %10 : $(Builtin.Int64, Builtin.Int1), 0
+  cond_fail %9 : $Builtin.Int1
+  %13 = builtin "sadd_with_overflow_Int64"(%7 : $Builtin.Int64, %2 : $Builtin.Int64, %1 : $Builtin.Int1) : $(Builtin.Int64, Builtin.Int1)
+  %14 = tuple_extract %13 : $(Builtin.Int64, Builtin.Int1), 0
+  %15 = tuple_extract %13 : $(Builtin.Int64, Builtin.Int1), 1
+  cond_fail %15 : $Builtin.Int1
+  %17 = builtin "cmp_eq_Int64"(%14 : $Builtin.Int64, %3 : $Builtin.Int64) : $Builtin.Int1
+  %18 = builtin "cmp_eq_Int64"(%11 : $Builtin.Int64, %3 : $Builtin.Int64) : $Builtin.Int1
+  cond_br %18, bb3, bb1(%14 : $Builtin.Int64, %11 : $Builtin.Int64, %17 : $Builtin.Int1)
+
+// Make sure there can not be any cond_fail here.
+// CHECK: bb2:
+// CHECK-NEXT: br bb1
+bb2:
+  br bb1(%2 : $Builtin.Int64, %2 : $Builtin.Int64, %1 : $Builtin.Int1)
+
+// CHECK: bb3:
+bb3:
+  %5 = tuple()
+  return %5 : $()
+}