Fix LICM to avoid hoisting never-executed traps

It is legal for the optimizer to consider code after a loop always
reachable, but when a loop has no exits, or when the loops exits are
dominated by a conditional statement, we should not consider
conditional statements within the loop as dominating all possible
execution paths through the loop. At least not when there is at least
one path through the loop that contains a "synchronization point",
such as a function that may contain a memory barrier, perform I/O, or
exit the program.

Sadly, we still don't model synchronization points in the optimizer,
so we need to conservatively assume all loops have a synchronization
point and avoid hoisting conditional traps that may never be executed.

Fixes rdar://66791257 (Print statement provokes "Can't unsafeBitCast
between types of different sizes" when optimizations enabled)

Originated in 2014.

Author: atrick

include/swift/SIL/LoopInfo.h

@@ -50,6 +50,16 @@ class SILLoop : public llvm::LoopBase<SILBasicBlock, SILLoop> {
   /// this loop by unrolling or versioning.
   bool canDuplicate(SILInstruction *Inst) const;
 
+  void getExitingAndLatchBlocks(
+    SmallVectorImpl<SILBasicBlock *> &ExitingAndLatchBlocks) const {
+    this->getExitingBlocks(ExitingAndLatchBlocks);
+    SILBasicBlock *header = getHeader();
+    for (auto *predBB : header->getPredecessorBlocks()) {
+      if (contains(predBB) && !this->isLoopExiting(predBB))
+        ExitingAndLatchBlocks.push_back(predBB);
+    }
+  }
+
 private:
   friend class llvm::LoopInfoBase<SILBasicBlock, SILLoop>;
 

lib/SILOptimizer/LoopTransforms/LICM.cpp

@@ -214,16 +214,16 @@ static void getDominatingBlocks(SmallVectorImpl<SILBasicBlock *> &domBlocks,
                                 SILLoop *Loop, DominanceInfo *DT) {
   auto HeaderBB = Loop->getHeader();
   auto DTRoot = DT->getNode(HeaderBB);
-  SmallVector<SILBasicBlock *, 8> ExitingBBs;
-  Loop->getExitingBlocks(ExitingBBs);
+  SmallVector<SILBasicBlock *, 8> ExitingAndLatchBBs;
+  Loop->getExitingAndLatchBlocks(ExitingAndLatchBBs);
   for (llvm::df_iterator<DominanceInfoNode *> It = llvm::df_begin(DTRoot),
                                               E = llvm::df_end(DTRoot);
        It != E;) {
     auto *CurBB = It->getBlock();
 
     // Don't decent into control-dependent code. Only traverse into basic blocks
     // that dominate all exits.
-    if (!std::all_of(ExitingBBs.begin(), ExitingBBs.end(),
+    if (!std::all_of(ExitingAndLatchBBs.begin(), ExitingAndLatchBBs.end(),
                      [=](SILBasicBlock *ExitBB) {
           return DT->dominates(CurBB, ExitBB);
         })) {

test/SILOptimizer/licm.sil

@@ -854,3 +854,82 @@ bb9(%39 : $Builtin.Int64):                        // Preds: bb7
   dealloc_stack %3 : $*Index           // id: %41
   return %40 : $Int64                               // id: %42
 }
+
+// testConditionalTrapInInfiniteSyncLoop and
+// testConditionalTrapDominatingSyncLoopExit
+//
+// It's legal for the optimizer to consider code after the loop as
+// always reachable, but when a loop has no exits, or when the loops
+// exits are dominated by a conditional statement we should not
+// consider conditional statements within the loop as dominating all
+// possible execution paths through the loop. At least not when there
+// is at least one path through the loop that contains a
+// "synchronization point", such as a function that may contain a
+// memory barrier, perform I/O, or exit the program.
+sil @mayExit : $@convention(thin) () -> ()
+
+// CHECK-LABEL: sil @testConditionalTrapInInfiniteSyncLoop : $@convention(thin) (Builtin.Int1, Builtin.Int1) -> () {
+// CHECK: bb0
+// CHECK-NOT: cond_fail
+// CHECK:   br bb1
+// CHECK: bb1:
+// CHECK:   cond_br %0, bb2, bb3
+// CHECK: bb2:
+// CHECK: cond_fail %1 : $Builtin.Int1, "arithmetic overflow"
+// CHECK-LABEL: } // end sil function 'testConditionalTrapInInfiniteSyncLoop'
+sil @testConditionalTrapInInfiniteSyncLoop : $@convention(thin) (Builtin.Int1, Builtin.Int1) -> () {
+bb0(%0 : $Builtin.Int1, %1 : $Builtin.Int1):
+  br bb1
+
+bb1: // loop head
+  cond_br %0, bb2, bb3
+
+bb2: // maybe never executed
+  cond_fail %1 : $Builtin.Int1, "arithmetic overflow"
+  br bb4
+
+bb3:
+  // synchronization point: has "real" side-effects that we can't
+  // reorder with traps
+  %f = function_ref @mayExit : $@convention(thin) () -> ()
+  apply %f() : $@convention(thin) () -> ()
+  br bb4
+
+bb4: // latch
+  br bb1
+}
+
+// CHECK-LABEL: sil @testConditionalTrapDominatingSyncLoopExit : $@convention(thin) (Builtin.Int1, Builtin.Int1, Builtin.Int1) -> () {
+// CHECK: bb0
+// CHECK-NOT: cond_fail
+// CHECK:   br bb1
+// CHECK: bb1:
+// CHECK:   cond_br %0, bb2, bb4
+// CHECK: bb2:
+// CHECK:   cond_fail %1 : $Builtin.Int1, "arithmetic overflow"
+// CHECK-LABEL: } // end sil function 'testConditionalTrapDominatingSyncLoopExit'
+sil @testConditionalTrapDominatingSyncLoopExit : $@convention(thin) (Builtin.Int1, Builtin.Int1, Builtin.Int1) -> () {
+bb0(%0 : $Builtin.Int1, %1 : $Builtin.Int1, %2 : $Builtin.Int1):
+  br bb1
+
+bb1: // loop head
+  cond_br %0, bb2, bb4
+
+bb2: // maybe never executed
+  cond_fail %1 : $Builtin.Int1, "arithmetic overflow"
+  cond_br %2, bb3, bb5
+
+bb3: // tail
+  br bb1
+
+bb4:
+  // synchronization point: has "real" side-effects that we can't
+  // reorder with traps
+  %f = function_ref @mayExit : $@convention(thin) () -> ()
+  apply %f() : $@convention(thin) () -> ()
+  br bb1
+
+bb5:
+  %99 = tuple ()
+  return %99 : $()
+}