[LICM] Add support for Hosting <Instruction, Instruction Set> Pairs

Joe Shajrawi · Joe Shajrawi · commit f56b5d87307a · 2018-06-27T16:25:35.000-07:00
Support having the target of each hoist instruction as multiple sinks.
diff --git a/lib/SILOptimizer/LoopTransforms/LICM.cpp b/lib/SILOptimizer/LoopTransforms/LICM.cpp
@@ -40,12 +40,13 @@ using namespace swift;
 
 /// Instructions which can be hoisted:
 /// loads, function calls without side effects and (some) exclusivity checks
-using HoistSet = llvm::SmallPtrSet<SILInstruction *, 8>;
+using InstSet = llvm::SmallPtrSet<SILInstruction *, 8>;
 
 /// Instruction pairs which need to be hoisted together:
 /// e.g. If we hoist a begin access, we need to sink the matching end access
-using HoistPairSet =
-    llvm::SmallVector<std::pair<SILInstruction *, SILInstruction *>, 8>;
+/// The target of each hoist instruction can be multiple sinks. As such,
+/// A pair consists of an instruction to a set of matching sink instructions
+using HoistPairSet = llvm::SmallVector<std::pair<SILInstruction *, InstSet>, 8>;
 
 /// A subset of instruction which may have side effects.
 /// Doesn't contain ones that have special handling (e.g. fix_lifetime)
@@ -149,6 +150,7 @@ static bool hoistInstruction(DominanceInfo *DT, SILInstruction *Inst,
     DEBUG(llvm::dbgs() << "   loop variant operands\n");
     return false;
   }
+
   auto mvBefore = Preheader->getTerminator();
   ArraySemanticsCall semCall(Inst);
   if (semCall.canHoist(mvBefore, DT)) {
@@ -160,7 +162,7 @@ static bool hoistInstruction(DominanceInfo *DT, SILInstruction *Inst,
 }
 
 static bool hoistInstructions(SILLoop *Loop, DominanceInfo *DT,
-                              HoistSet &HoistUpSet) {
+                              InstSet &HoistUpSet) {
   DEBUG(llvm::dbgs() << " Hoisting instructions.\n");
   auto Preheader = Loop->getLoopPreheader();
   assert(Preheader && "Expected a preheader");
@@ -271,7 +273,7 @@ static bool sinkInstruction(DominanceInfo *DT,
 
 static bool sinkInstructions(std::unique_ptr<LoopNestSummary> &LoopSummary,
                              DominanceInfo *DT, SILLoopInfo *LI,
-                             HoistSet &SinkDownSet) {
+                             InstSet &SinkDownSet) {
   auto *Loop = LoopSummary->Loop;
   DEBUG(llvm::errs() << " Sink instructions attempt\n");
   SmallVector<SILBasicBlock *, 8> domBlocks;
@@ -300,15 +302,18 @@ hoistAndSinkInstructionPairs(std::unique_ptr<LoopNestSummary> &LoopSummary,
   assert(Preheader && "Expected a preheader");
 
   bool Changed = false;
+
   for (auto pair : Pairs) {
     auto *Up = pair.first;
-    auto *Down = pair.second;
+    auto &SinkDownSet = pair.second;
     if (!hoistInstruction(DT, Up, Loop, Preheader)) {
       continue;
     }
     DEBUG(llvm::dbgs() << "Hoisted " << *Up);
-    if (!sinkInstruction(DT, LoopSummary, Down, LI)) {
-      llvm_unreachable("LICM: Could not perform must-sink instruction");
+    for (auto *instSink : SinkDownSet) {
+      if (!sinkInstruction(DT, LoopSummary, instSink, LI)) {
+        llvm_unreachable("LICM: Could not perform must-sink instruction");
+      }
     }
     DEBUG(llvm::errs() << " Successfully hosited and sank pair\n");
     Changed = true;
@@ -333,10 +338,10 @@ class LoopTreeOptimization {
   bool RunsOnHighLevelSIL;
 
   /// Instructions that we may be able to hoist up
-  HoistSet HoistUp;
+  InstSet HoistUp;
 
   /// Instructions that we may be able to sink down
-  HoistSet SinkDown;
+  InstSet SinkDown;
 
   /// Instruction pairs that we may be able to hoist and sink
   HoistPairSet HoistingPairs;
@@ -462,7 +467,26 @@ static bool canHoistUpDefault(SILInstruction *inst, SILLoop *Loop,
   return semCall.canHoist(Preheader->getTerminator(), DT);
 }
 
-static void analyzeBeginAccess(BeginAccessInst *&BI,
+// Check If all the end accesses of the given begin do not prevent hoisting
+// There are only two legal placements for the end access instructions:
+// 1) Inside the same loop (sink to loop exists)
+// Potential TODO: At loop exit block
+static bool handledEndAccesses(BeginAccessInst *BI, SILLoop *Loop) {
+  for (auto Use : BI->getUses()) {
+    auto *User = Use->getUser();
+    if (!dyn_cast<EndAccessInst>(User)) {
+      continue;
+    }
+    auto *BB = User->getParent();
+    if (Loop->getBlocksSet().count(BB) != 0) {
+      continue;
+    }
+    return false;
+  }
+  return true;
+}
+
+static void analyzeBeginAccess(BeginAccessInst *BI,
                                SmallVector<BeginAccessInst *, 8> &BeginAccesses,
                                SmallVector<EndAccessInst *, 8> &EndAccesses,
                                HoistPairSet &HoistingPairs) {
@@ -476,23 +500,26 @@ static void analyzeBeginAccess(BeginAccessInst *&BI,
     return;
   }
 
-  // find matching end access:
+  // find matching end accesses:
+  InstSet matchingEnds;
   auto matchingEndPred = [&](EndAccessInst *EI) {
     return EI->getBeginAccess() == BI;
   };
-  auto matchingEnd =
-      std::find_if(EndAccesses.begin(), EndAccesses.end(), matchingEndPred);
-  if (matchingEnd == EndAccesses.end()) {
-    // no matching end within the loop
-    return;
+  for (auto matchingEnd = std::find_if(EndAccesses.begin(), EndAccesses.end(),
+                                       matchingEndPred);
+       matchingEnd != EndAccesses.end();
+       matchingEnd =
+           std::find_if(matchingEnd, EndAccesses.end(), matchingEndPred)) {
+    auto *EI = *matchingEnd;
+    matchingEnds.insert(EI);
+    ++matchingEnd;
   }
-  auto *EI = *matchingEnd;
-  ++matchingEnd;
-  if (std::find_if(matchingEnd, EndAccesses.end(), matchingEndPred) !=
-      EndAccesses.end()) {
-    // We expect a single matching end access in the loop
+  if (matchingEnds.empty()) {
+    // no matching end within the loop
     return;
   }
+  DEBUG(llvm::dbgs() << "Found " << matchingEnds.size() << " End accesses"
+                     << "\n");
 
   auto BIAccessedStorageNonNested = findAccessedStorageNonNested(BI);
   auto safeBeginPred = [&](BeginAccessInst *OtherBI) {
@@ -504,7 +531,7 @@ static void analyzeBeginAccess(BeginAccessInst *&BI,
   };
 
   if (std::all_of(BeginAccesses.begin(), BeginAccesses.end(), safeBeginPred)) {
-    HoistingPairs.push_back(std::make_pair(BI, EI));
+    HoistingPairs.emplace_back(std::make_pair(BI, std::move(matchingEnds)));
   }
 }
 
@@ -618,6 +645,12 @@ void LoopTreeOptimization::analyzeCurrentLoop(
     }
   }
   for (auto *BI : BeginAccesses) {
+    if (!handledEndAccesses(BI, Loop)) {
+      DEBUG(llvm::dbgs() << "Skipping: " << *BI);
+      DEBUG(llvm::dbgs() << "Some end accesses can't be handled"
+                         << "\n");
+      continue;
+    }
     analyzeBeginAccess(BI, BeginAccesses, EndAccesses, HoistingPairs);
   }
 }
diff --git a/test/SILOptimizer/licm_multiend.sil b/test/SILOptimizer/licm_multiend.sil
@@ -0,0 +1,214 @@
+// RUN: %target-sil-opt -assume-parsing-unqualified-ownership-sil -enable-sil-verify-all %s -licm | %FileCheck %s
+// REQUIRES: CPU=x86_64
+// REQUIRES: OS=macosx
+
+sil_stage canonical
+
+import Builtin
+import Swift
+import SwiftShims
+
+var x: Int
+
+let reversedArray: ReversedCollection<[Int]>
+
+// x
+sil_global hidden @$S3tmp1xSivp : $Int
+
+// reversedArray
+sil_global hidden [let] @$S3tmp13reversedArrays18ReversedCollectionVySaySiGGvp : $ReversedCollection<Array<Int>>
+
+// _swiftEmptyArrayStorage
+sil_global @_swiftEmptyArrayStorage : $_SwiftEmptyArrayStorage
+
+
+// CHECK-LABEL: sil hidden @multi_end_licm : $@convention(thin) () -> () {
+// CHECK: bb2:
+// CHECK: [[GLOBALVAR:%.*]] = global_addr @$S3tmp1xSivp : $*Int
+// CHECK: [[BEGINA:%.*]] = begin_access [modify] [dynamic] [no_nested_conflict] [[GLOBALVAR]] : $*Int
+// CHECK-NEXT: br [[LOOPH:bb[0-9]+]]({{.*}} : $Builtin.Int64)
+// CHECK: [[LOOPH]]({{.*}} : $Builtin.Int64)
+// CHECK: cond_br {{.*}}, [[LOOPCOND1:bb[0-9]+]], [[LOOPCOND2:bb[0-9]+]]
+// CHECK: [[LOOPCOND1]]:
+// CHECK-NEXT: store
+// CHECK-NEXT: cond_br {{.*}}, [[LOOPEXIT1:bb[0-9]+]], [[LOOPCONT1:bb[0-9]+]]
+// CHECK: [[LOOPEXIT1]]:
+// CHECK-NEXT: end_access [[BEGINA]] : $*Int
+// CHECK-NEXT: br [[LOOPAFTEREXIT:bb[0-9]+]]
+// CHECK: [[LOOPCOND2]]:
+// CHECK-NEXT: struct $Int
+// CHECK-NEXT: store
+// CHECK-NEXT: cond_br {{.*}}, [[LOOPEXIT2:bb[0-9]+]], [[LOOPCONT1]]
+// CHECK: [[LOOPEXIT2]]:
+// CHECK-NEXT: end_access [[BEGINA]] : $*Int
+// CHECK-NEXT: br [[LOOPAFTEREXIT]]
+// CHECK: [[LOOPCONT1]]:
+// CHECK-NEXT: br [[LOOPH]]
+// CHECK: [[LOOPAFTEREXIT]]:
+// CHECK-NEXT: br [[FUNCRET:bb[0-9]+]]
+// CHECK: [[FUNCRET]]:
+// CHECK-NEXT: tuple
+// CHECK-NEXT: return
+sil hidden @multi_end_licm : $@convention(thin) () -> () {
+bb0:
+  %0 = global_addr @$S3tmp13reversedArrays18ReversedCollectionVySaySiGGvp : $*ReversedCollection<Array<Int>>
+  %1 = struct_element_addr %0 : $*ReversedCollection<Array<Int>>, #ReversedCollection._base
+  %2 = struct_element_addr %1 : $*Array<Int>, #Array._buffer
+  %3 = struct_element_addr %2 : $*_ArrayBuffer<Int>, #_ArrayBuffer._storage
+  %4 = struct_element_addr %3 : $*_BridgeStorage<_ContiguousArrayStorageBase, _NSArrayCore>, #_BridgeStorage.rawValue
+  %5 = load %4 : $*Builtin.BridgeObject
+  %6 = unchecked_ref_cast %5 : $Builtin.BridgeObject to $_ContiguousArrayStorageBase
+  %7 = ref_element_addr %6 : $_ContiguousArrayStorageBase, #_ContiguousArrayStorageBase.countAndCapacity
+  %8 = struct_element_addr %7 : $*_ArrayBody, #_ArrayBody._storage
+  %9 = struct_element_addr %8 : $*_SwiftArrayBodyStorage, #_SwiftArrayBodyStorage.count
+  %10 = struct_element_addr %9 : $*Int, #Int._value
+  %11 = load %10 : $*Builtin.Int64
+  %12 = builtin "assumeNonNegative_Int64"(%11 : $Builtin.Int64) : $Builtin.Int64
+  %13 = integer_literal $Builtin.Int64, 0
+  %14 = integer_literal $Builtin.Int1, 0
+  %15 = builtin "cmp_eq_Int64"(%12 : $Builtin.Int64, %13 : $Builtin.Int64) : $Builtin.Int1
+  %16 = builtin "int_expect_Int1"(%15 : $Builtin.Int1, %14 : $Builtin.Int1) : $Builtin.Int1
+  cond_br %16, bb1, bb2
+
+bb1:
+  br bbRet
+
+bb2:
+  %19 = global_addr @$S3tmp1xSivp : $*Int
+  %20 = integer_literal $Builtin.Int64, 1
+  %21 = integer_literal $Builtin.Int1, -1
+  %23 = ref_tail_addr %6 : $_ContiguousArrayStorageBase, $Int
+  br bb4(%12 : $Builtin.Int64)
+
+bb4(%27 : $Builtin.Int64):
+  %28 = builtin "ssub_with_overflow_Int64"(%27 : $Builtin.Int64, %20 : $Builtin.Int64, %21 : $Builtin.Int1) : $(Builtin.Int64, Builtin.Int1)
+  %29 = tuple_extract %28 : $(Builtin.Int64, Builtin.Int1), 0
+  %30 = tuple_extract %28 : $(Builtin.Int64, Builtin.Int1), 1
+  cond_fail %30 : $Builtin.Int1
+  %32 = builtin "cmp_slt_Int64"(%29 : $Builtin.Int64, %13 : $Builtin.Int64) : $Builtin.Int1
+  %33 = load %10 : $*Builtin.Int64
+  %34 = builtin "assumeNonNegative_Int64"(%33 : $Builtin.Int64) : $Builtin.Int64 
+  %35 = builtin "cmp_slt_Int64"(%29 : $Builtin.Int64, %34 : $Builtin.Int64) : $Builtin.Int1
+  %36 = builtin "xor_Int1"(%35 : $Builtin.Int1, %21 : $Builtin.Int1) : $Builtin.Int1
+  %37 = builtin "or_Int1"(%32 : $Builtin.Int1, %36 : $Builtin.Int1) : $Builtin.Int1
+  cond_fail %37 : $Builtin.Int1
+  %39 = builtin "truncOrBitCast_Int64_Word"(%29 : $Builtin.Int64) : $Builtin.Word
+  %40 = index_addr %23 : $*Int, %39 : $Builtin.Word
+  %41 = struct_element_addr %40 : $*Int, #Int._value
+  %42 = load %41 : $*Builtin.Int64
+  %43 = struct $Int (%42 : $Builtin.Int64)
+  debug_value %43 : $Int, let, name "item"
+  %global = begin_access [modify] [dynamic] [no_nested_conflict] %19 : $*Int
+  %46 = builtin "cmp_eq_Int64"(%29 : $Builtin.Int64, %13 : $Builtin.Int64) : $Builtin.Int1
+  %47 = builtin "int_expect_Int1"(%46 : $Builtin.Int1, %14 : $Builtin.Int1) : $Builtin.Int1
+  cond_br %47, bbend1, bbend2
+  
+bbend1:
+  store %43 to %global : $*Int
+  end_access %global : $*Int
+  cond_br %47, bb6, bb5
+  
+bbend2:
+  %otherInt = struct $Int (%27 : $Builtin.Int64)
+  store %otherInt to %global : $*Int
+  end_access %global : $*Int
+  cond_br %47, bb6, bb5
+
+bb5:
+  br bb4(%29 : $Builtin.Int64)
+
+bb6:
+  br bbRet
+  
+bbRet:
+  %25 = tuple ()
+  return %25 : $()
+} // end sil function 'multi_end_licm'
+
+// CHECK-LABEL: sil hidden @multi_end_licm_loop_exit : $@convention(thin) () -> () {
+// CHECK: br [[LOOPH:bb[0-9]+]]({{.*}} : $Builtin.Int64)
+// CHECK: [[LOOPH]]({{.*}} : $Builtin.Int64)
+// CHECK: begin_access [modify] [dynamic] [no_nested_conflict]
+// CHECK: cond_br {{.*}}, [[LOOPCOND1:bb[0-9]+]], [[LOOPCOND2:bb[0-9]+]]
+// CHECK: [[LOOPCOND1]]
+// CHECK-NEXT: store
+// CHECK-NEXT: end_access
+// CHECK: return
+sil hidden @multi_end_licm_loop_exit : $@convention(thin) () -> () {
+bb0:
+  %0 = global_addr @$S3tmp13reversedArrays18ReversedCollectionVySaySiGGvp : $*ReversedCollection<Array<Int>>
+  %1 = struct_element_addr %0 : $*ReversedCollection<Array<Int>>, #ReversedCollection._base
+  %2 = struct_element_addr %1 : $*Array<Int>, #Array._buffer
+  %3 = struct_element_addr %2 : $*_ArrayBuffer<Int>, #_ArrayBuffer._storage
+  %4 = struct_element_addr %3 : $*_BridgeStorage<_ContiguousArrayStorageBase, _NSArrayCore>, #_BridgeStorage.rawValue
+  %5 = load %4 : $*Builtin.BridgeObject
+  %6 = unchecked_ref_cast %5 : $Builtin.BridgeObject to $_ContiguousArrayStorageBase
+  %7 = ref_element_addr %6 : $_ContiguousArrayStorageBase, #_ContiguousArrayStorageBase.countAndCapacity
+  %8 = struct_element_addr %7 : $*_ArrayBody, #_ArrayBody._storage
+  %9 = struct_element_addr %8 : $*_SwiftArrayBodyStorage, #_SwiftArrayBodyStorage.count
+  %10 = struct_element_addr %9 : $*Int, #Int._value
+  %11 = load %10 : $*Builtin.Int64
+  %12 = builtin "assumeNonNegative_Int64"(%11 : $Builtin.Int64) : $Builtin.Int64
+  %13 = integer_literal $Builtin.Int64, 0
+  %14 = integer_literal $Builtin.Int1, 0
+  %15 = builtin "cmp_eq_Int64"(%12 : $Builtin.Int64, %13 : $Builtin.Int64) : $Builtin.Int1
+  %16 = builtin "int_expect_Int1"(%15 : $Builtin.Int1, %14 : $Builtin.Int1) : $Builtin.Int1
+  cond_br %16, bb1, bb2
+
+bb1:
+  br bbRet
+
+bb2:
+  %19 = global_addr @$S3tmp1xSivp : $*Int
+  %20 = integer_literal $Builtin.Int64, 1
+  %21 = integer_literal $Builtin.Int1, -1
+  %23 = ref_tail_addr %6 : $_ContiguousArrayStorageBase, $Int
+  br bb4(%12 : $Builtin.Int64)
+
+bb4(%27 : $Builtin.Int64):
+  %28 = builtin "ssub_with_overflow_Int64"(%27 : $Builtin.Int64, %20 : $Builtin.Int64, %21 : $Builtin.Int1) : $(Builtin.Int64, Builtin.Int1)
+  %29 = tuple_extract %28 : $(Builtin.Int64, Builtin.Int1), 0
+  %30 = tuple_extract %28 : $(Builtin.Int64, Builtin.Int1), 1
+  cond_fail %30 : $Builtin.Int1
+  %32 = builtin "cmp_slt_Int64"(%29 : $Builtin.Int64, %13 : $Builtin.Int64) : $Builtin.Int1
+  %33 = load %10 : $*Builtin.Int64
+  %34 = builtin "assumeNonNegative_Int64"(%33 : $Builtin.Int64) : $Builtin.Int64 
+  %35 = builtin "cmp_slt_Int64"(%29 : $Builtin.Int64, %34 : $Builtin.Int64) : $Builtin.Int1
+  %36 = builtin "xor_Int1"(%35 : $Builtin.Int1, %21 : $Builtin.Int1) : $Builtin.Int1
+  %37 = builtin "or_Int1"(%32 : $Builtin.Int1, %36 : $Builtin.Int1) : $Builtin.Int1
+  cond_fail %37 : $Builtin.Int1
+  %39 = builtin "truncOrBitCast_Int64_Word"(%29 : $Builtin.Int64) : $Builtin.Word
+  %40 = index_addr %23 : $*Int, %39 : $Builtin.Word
+  %41 = struct_element_addr %40 : $*Int, #Int._value
+  %42 = load %41 : $*Builtin.Int64
+  %43 = struct $Int (%42 : $Builtin.Int64)
+  debug_value %43 : $Int, let, name "item"
+  %global = begin_access [modify] [dynamic] [no_nested_conflict] %19 : $*Int
+  %46 = builtin "cmp_eq_Int64"(%29 : $Builtin.Int64, %13 : $Builtin.Int64) : $Builtin.Int1
+  %47 = builtin "int_expect_Int1"(%46 : $Builtin.Int1, %14 : $Builtin.Int1) : $Builtin.Int1
+  cond_br %47, bbend1, bbend2
+  
+bbend1:
+  store %43 to %global : $*Int
+  end_access %global : $*Int
+  cond_br %47, bb6, bb5
+  
+bbend2:
+  %otherInt = struct $Int (%27 : $Builtin.Int64)
+  store %otherInt to %global : $*Int
+  cond_br %47, bbOut, bb5
+
+bbOut:
+  end_access %global : $*Int
+  br bb6
+
+bb5:
+  br bb4(%29 : $Builtin.Int64)
+
+bb6:
+  br bbRet
+  
+bbRet:
+  %25 = tuple ()
+  return %25 : $()
+} // end sil function 'multi_end_licm_loop_exit'
