Sink address projections in ArrayPropertyOpt so that there are no address phis

Author: meg-gupta

lib/SILOptimizer/LoopTransforms/ArrayPropertyOpt.cpp

@@ -53,19 +53,19 @@
 #define DEBUG_TYPE "array-property-opt"
 
 #include "ArrayOpt.h"
+#include "swift/SIL/CFG.h"
+#include "swift/SIL/DebugUtils.h"
+#include "swift/SIL/InstructionUtils.h"
+#include "swift/SIL/LoopInfo.h"
+#include "swift/SIL/Projection.h"
+#include "swift/SIL/SILCloner.h"
 #include "swift/SILOptimizer/Analysis/ArraySemantic.h"
 #include "swift/SILOptimizer/Analysis/LoopAnalysis.h"
 #include "swift/SILOptimizer/PassManager/Transforms.h"
+#include "swift/SILOptimizer/Utils/BasicBlockOptUtils.h"
 #include "swift/SILOptimizer/Utils/CFGOptUtils.h"
 #include "swift/SILOptimizer/Utils/LoopUtils.h"
 #include "swift/SILOptimizer/Utils/SILSSAUpdater.h"
-#include "swift/SIL/CFG.h"
-#include "swift/SIL/DebugUtils.h"
-#include "swift/SIL/InstructionUtils.h"
-#include "swift/SIL/Projection.h"
-#include "swift/SIL/LoopInfo.h"
-#include "swift/SIL/BasicBlockBits.h"
-#include "swift/SIL/SILCloner.h"
 #include "llvm/ADT/SmallSet.h"
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/Debug.h"
@@ -86,6 +86,8 @@ class ArrayPropertiesAnalysis {
   SILBasicBlock *Preheader;
   DominanceInfo *DomTree;
 
+  SinkAddressProjections sinkProj;
+
   llvm::DenseMap<SILFunction *, uint32_t> InstCountCache;
   llvm::SmallSet<SILValue, 16> HoistableArray;
 
@@ -169,13 +171,18 @@ class ArrayPropertiesAnalysis {
 
     bool FoundHoistable = false;
     uint32_t LoopInstCount = 0;
+
     for (auto *BB : Loop->getBlocks()) {
       for (auto &Inst : *BB) {
         // Can't clone alloc_stack instructions whose dealloc_stack is outside
         // the loop.
         if (!canDuplicateLoopInstruction(Loop, &Inst))
           return false;
 
+        if (!sinkProj.analyzeAddressProjections(&Inst)) {
+          return false;
+        }
+
         ArraySemanticsCall ArrayPropsInst(&Inst, "array.props", true);
         if (!ArrayPropsInst)
           continue;
@@ -512,10 +519,11 @@ class RegionCloner : public SILCloner<RegionCloner> {
                          SILSSAUpdater &SSAUp) {
     // Collect outside uses.
     SmallVector<UseWrapper, 16> UseList;
-    for (auto Use : V->getUses())
+    for (auto Use : V->getUses()) {
       if (!isBlockCloned(Use->getUser()->getParent())) {
         UseList.push_back(UseWrapper(Use));
       }
+    }
     if (UseList.empty())
       return;
 
@@ -532,15 +540,40 @@ class RegionCloner : public SILCloner<RegionCloner> {
 
   void updateSSAForm() {
     SILSSAUpdater SSAUp;
+    SmallVector<SingleValueInstruction *, 4> newProjections;
+    SinkAddressProjections sinkProj(&newProjections);
+
     for (auto *origBB : originalPreorderBlocks()) {
       // Update outside used phi values.
-      for (auto *arg : origBB->getArguments())
+      for (auto *arg : origBB->getArguments()) {
         updateSSAForValue(origBB, arg, SSAUp);
+      }
 
       // Update outside used instruction values.
       for (auto &inst : *origBB) {
-        for (auto result : inst.getResults())
-          updateSSAForValue(origBB, result, SSAUp);
+        for (auto result : inst.getResults()) {
+          bool success = sinkProj.analyzeAddressProjections(&inst);
+          assert(success);
+          // Sink address projections by cloning to avoid address phis.
+          sinkProj.cloneProjections();
+
+          // If no new projections were created, update ssa for the result only.
+          if (newProjections.empty()) {
+            updateSSAForValue(origBB, result, SSAUp);
+            continue;
+          }
+
+          for (auto *newProj : newProjections) {
+            // Operand values of new projections may need ssa update.
+            for (auto opVal : newProj->getOperandValues()) {
+              if (!isBlockCloned(opVal->getParentBlock())) {
+                continue;
+              }
+              updateSSAForValue(origBB, opVal, SSAUp);
+            }
+          }
+          newProjections.clear();
+        }
       }
     }
   }

test/SILOptimizer/array_property_opt.sil

@@ -1,5 +1,9 @@
 // RUN: %target-sil-opt -parse-serialized-sil -enable-sil-verify-all %s -array-property-opt | %FileCheck %s
 
+// Linux doesn't have the same symbol name for _ArrayBuffer, which is part of
+// the ObjC runtime interop.  Use `_ContiguousArrayBuffer instead.
+// REQUIRES: objc_interop
+
 sil_stage canonical
 
 import Builtin
@@ -258,3 +262,132 @@ bb10: // Exit dominated by bb3
 bb11: // Non-exit dominated by bb1
   return %4 : $Builtin.Int1
 }
+
+class Klass {
+  var val: Optional<Int>
+}
+
+struct WrapperStruct {
+  var val: Klass
+}
+
+sil @use_klass : $@convention(thin) (@in_guaranteed Klass) -> ()
+
+sil @test_sink_address_proj : $@convention(thin) (@inout MyArray<MyClass>, @in_guaranteed WrapperStruct) -> () {
+bb0(%0 : $*MyArray<MyClass>, %1 : $*WrapperStruct):
+  %3 = load %0 : $*MyArray<MyClass>
+  br bb1
+
+bb1:
+  %2 = function_ref @arrayPropertyIsNative : $@convention(method) (@owned MyArray<MyClass>) -> Bool
+  retain_value %3 : $MyArray<MyClass>
+  %5 = apply %2(%3) : $@convention(method) (@owned MyArray<MyClass>) -> Bool
+  %ele = struct_element_addr %1 : $*WrapperStruct, #WrapperStruct.val
+  cond_br undef, bb5, bb2
+
+bb2:
+ %6 = integer_literal $Builtin.Int1, -1
+ cond_br %6, bb3, bb4
+
+bb3:
+  br bb1
+
+bb4:
+ br bb6
+
+bb5:
+ %f = function_ref @use_klass : $@convention(thin) (@in_guaranteed Klass) -> ()
+ %a = apply %f(%ele) : $@convention(thin) (@in_guaranteed Klass) -> ()
+ br bb6
+
+bb6:
+ %t = tuple ()
+ return %t : $()
+}
+
+sil [_semantics "array.props.isNativeTypeChecked"] @hoistableIsNativeTypeChecked : $@convention(method) (@guaranteed Array<Klass>) -> Bool
+sil [_semantics "array.get_element"] @getElement : $@convention(method) (Int, Bool, _DependenceToken, @guaranteed Array<Klass>) -> @owned Klass
+sil [_semantics "array.get_count"] @getCount : $@convention(method) (@guaranteed Array<Klass>) -> Int
+
+sil hidden @test_array_prop_opt : $@convention(thin) (@guaranteed Optional<Array<Klass>>) -> Int {
+bb0(%0 : $Optional<Array<Klass>>):
+  %4 = integer_literal $Builtin.Int64, 0
+  switch_enum %0 : $Optional<Array<Klass>>, case #Optional.some!enumelt: bb2, case #Optional.none!enumelt: bb1
+
+bb1:
+  br bb12(%4 : $Builtin.Int64)
+
+bb2(%12 : $Array<Klass>):
+  %14 = function_ref @getCount : $@convention(method) (@guaranteed Array<Klass>) -> Int
+  retain_value %0 : $Optional<Array<Klass>>
+  retain_value %0 : $Optional<Array<Klass>>
+  %17 = apply %14(%12) : $@convention(method) (@guaranteed Array<Klass>) -> Int
+  %18 = struct_extract %17 : $Int, #Int._value
+  %19 = builtin "cmp_eq_Int64"(%18 : $Builtin.Int64, %4 : $Builtin.Int64) : $Builtin.Int1
+  cond_br %19, bb3, bb4
+
+bb3:
+  release_value %0 : $Optional<Array<Klass>>
+  %22 = unchecked_enum_data %0 : $Optional<Array<Klass>>, #Optional.some!enumelt
+  %23 = struct_extract %22 : $Array<Klass>, #Array._buffer
+  %24 = struct_extract %23 : $_ArrayBuffer<Klass>, #_ArrayBuffer._storage
+  %25 = struct_extract %24 : $_BridgeStorage<__ContiguousArrayStorageBase>, #_BridgeStorage.rawValue
+  strong_release %25 : $Builtin.BridgeObject
+  br bb12(%4 : $Builtin.Int64)
+
+bb4:
+  %28 = function_ref @hoistableIsNativeTypeChecked : $@convention(method) (@guaranteed Array<Klass>) -> Bool
+  %29 = function_ref @getElement : $@convention(method) (Int, Bool, _DependenceToken, @guaranteed Array<Klass>) -> @owned Klass
+  %30 = integer_literal $Builtin.Int64, 1
+  %31 = integer_literal $Builtin.Int1, -1
+  %32 = struct $_DependenceToken ()
+  br bb5(%4 : $Builtin.Int64)
+
+bb5(%34 : $Builtin.Int64):
+  %35 = struct $Int (%34 : $Builtin.Int64)
+  %36 = apply %28(%12) : $@convention(method) (@guaranteed Array<Klass>) -> Bool
+  %37 = apply %29(%35, %36, %32, %12) : $@convention(method) (Int, Bool, _DependenceToken, @guaranteed Array<Klass>) -> @owned Klass
+  %38 = builtin "sadd_with_overflow_Int64"(%34 : $Builtin.Int64, %30 : $Builtin.Int64, %31 : $Builtin.Int1) : $(Builtin.Int64, Builtin.Int1)
+  %39 = tuple_extract %38 : $(Builtin.Int64, Builtin.Int1), 0
+  %40 = tuple_extract %38 : $(Builtin.Int64, Builtin.Int1), 1
+  cond_fail %40 : $Builtin.Int1, "arithmetic overflow"
+  %43 = ref_element_addr %37 : $Klass, #Klass.val
+  %44 = begin_access [read] [dynamic] [no_nested_conflict] %43 : $*Optional<Int>
+  %45 = load %44 : $*Optional<Int>
+  end_access %44 : $*Optional<Int>
+  switch_enum %45 : $Optional<Int>, case #Optional.some!enumelt: bb9, case #Optional.none!enumelt: bb6
+
+bb6:
+  strong_release %37 : $Klass
+  %49 = builtin "cmp_eq_Int64"(%39 : $Builtin.Int64, %18 : $Builtin.Int64) : $Builtin.Int1
+  cond_br %49, bb7, bb8
+
+bb7:
+  release_value %0 : $Optional<Array<Klass>>
+  release_value %0 : $Optional<Array<Klass>>
+  br bb12(%4 : $Builtin.Int64)
+
+bb8:
+  br bb5(%39 : $Builtin.Int64)
+
+bb9:
+  release_value %0 : $Optional<Array<Klass>>
+  %57 = begin_access [read] [dynamic] [no_nested_conflict] %43 : $*Optional<Int>
+  %58 = load %57 : $*Optional<Int>
+  end_access %57 : $*Optional<Int>
+  switch_enum %58 : $Optional<Int>, case #Optional.some!enumelt: bb11, case #Optional.none!enumelt: bb10
+
+bb10:
+  cond_fail %31 : $Builtin.Int1, "Unexpectedly found nil while unwrapping an Optional value"
+  unreachable
+
+bb11(%63 : $Int):
+  release_value %0 : $Optional<Array<Klass>>
+  strong_release %37 : $Klass
+  %66 = struct_extract %63 : $Int, #Int._value
+  br bb12(%66 : $Builtin.Int64)
+
+bb12(%69 : $Builtin.Int64):
+  %70 = struct $Int (%69 : $Builtin.Int64)
+  return %70 : $Int
+}