Merge pull request #19181 from eeckstein/array-opts

SILOptimizer: update bounds check and uniqueness check hoisting optimizations for using _modify in Array subscript.

Author: eeckstein

include/swift/SILOptimizer/Utils/Local.h

@@ -123,6 +123,9 @@ ProjectBoxInst *getOrCreateProjectBox(AllocBoxInst *ABI, unsigned Index);
 /// 'Self' to a generic argument of the callee.
 bool mayBindDynamicSelf(SILFunction *F);
 
+/// Check whether the \p addr is an address of a tail-allocated array element.
+bool isAddressOfArrayElement(SILValue addr);
+
 /// \brief Move an ApplyInst's FuncRef so that it dominates the call site.
 void placeFuncRef(ApplyInst *AI, DominanceInfo *DT);
 

lib/SILOptimizer/IPO/UsePrespecialized.cpp

@@ -148,7 +148,7 @@ bool UsePrespecialized::replaceByPrespecialized(SILFunction &F) {
     } else if (auto oldPApply = dyn_cast<PartialApplyInst>(AI)) {
       oldPApply->replaceAllUsesWith(cast<PartialApplyInst>(NewAI));
     } else {
-      assert(isa<TryApplyInst>(NewAI));
+      assert(isa<TryApplyInst>(NewAI) || isa<BeginApplyInst>(NewAI));
     }
     recursivelyDeleteTriviallyDeadInstructions(AI.getInstruction(), true);
     Changed = true;

lib/SILOptimizer/LoopTransforms/ArrayBoundsCheckOpts.cpp

@@ -87,42 +87,6 @@ static SILValue getArrayStructPointer(ArrayCallKind K, SILValue Array) {
   return Array;
 }
 
-/// Check whether the store is to the address obtained from a getElementAddress
-/// semantic call.
-///  %40 = function_ref @getElementAddress
-///  %42 = apply %40(%28, %37)
-///  %43 = struct_extract %42
-///  %44 = pointer_to_address strict %43
-///  store %1 to %44 : $*Int
-static bool isArrayEltStore(StoreInst *SI) {
-  // Strip the MarkDependenceInst (new array implementation) where the above
-  // pattern looks like the following.
-  // %40 = function_ref @getElementAddress
-  // %41 = apply %40(%21, %35)
-  // %42 = struct_element_addr %0 : $*Array<Int>, #Array._buffer
-  // %43 = struct_element_addr %42 : $*_ArrayBuffer<Int>, #_ArrayBuffer._storage
-  // %44 = struct_element_addr %43 : $*_BridgeStorage
-  // %45 = load %44 : $*Builtin.BridgeObject
-  // %46 = unchecked_ref_cast %45 : $... to $_ContiguousArrayStorageBase
-  // %47 = unchecked_ref_cast %46 : $... to $Builtin.NativeObject
-  // %48 = struct_extract %41 : $..., #UnsafeMutablePointer._rawValue
-  // %49 = pointer_to_address %48 : $Builtin.RawPointer to strict $*Int
-  // %50 = mark_dependence %49 : $*Int on %47 : $Builtin.NativeObject
-  // store %1 to %50 : $*Int
-  SILValue Dest = SI->getDest();
-  if (auto *MD = dyn_cast<MarkDependenceInst>(Dest))
-    Dest = MD->getOperand(0);
-
-  if (auto *PtrToAddr =
-          dyn_cast<PointerToAddressInst>(stripAddressProjections(Dest)))
-    if (auto *SEI = dyn_cast<StructExtractInst>(PtrToAddr->getOperand())) {
-      ArraySemanticsCall Call(SEI->getOperand());
-      if (Call && Call.getKind() == ArrayCallKind::kGetElementAddress)
-        return true;
-    }
-  return false;
-}
-
 static bool isReleaseSafeArrayReference(SILValue Ref,
                                         ArraySet &ReleaseSafeArrayReferences,
                                         RCIdentityFunctionInfo *RCIA) {
@@ -185,7 +149,7 @@ mayChangeArraySize(SILInstruction *I, ArrayCallKind &Kind, SILValue &Array,
   // stored in a runtime allocated object sub field of an alloca.
   if (auto *SI = dyn_cast<StoreInst>(I)) {
     auto Ptr = SI->getDest();
-    return isa<AllocStackInst>(Ptr) || isArrayEltStore(SI)
+    return isa<AllocStackInst>(Ptr) || isAddressOfArrayElement(SI->getDest())
                ? ArrayBoundsEffect::kNone
                : ArrayBoundsEffect::kMayChangeAny;
   }

lib/SILOptimizer/LoopTransforms/COWArrayOpt.cpp

@@ -798,20 +798,6 @@ bool COWArrayOpt::checkSafeElementValueUses(UserOperList &ElementValueUsers) {
   }
   return true;
 }
-static bool isArrayEltStore(StoreInst *SI) {
-  SILValue Dest = stripAddressProjections(SI->getDest());
-  if (auto *MD = dyn_cast<MarkDependenceInst>(Dest))
-    Dest = MD->getValue();
-
-  if (auto *PtrToAddr =
-          dyn_cast<PointerToAddressInst>(stripAddressProjections(Dest)))
-    if (auto *SEI = dyn_cast<StructExtractInst>(PtrToAddr->getOperand())) {
-      ArraySemanticsCall Call(SEI->getOperand());
-      if (Call && Call.getKind() == ArrayCallKind::kGetElementAddress)
-        return true;
-    }
-  return false;
-}
 
 /// Check whether the array semantic operation could change an array value to
 /// not be uniquely referenced.
@@ -1278,7 +1264,7 @@ bool COWArrayOpt::hoistInLoopWithOnlyNonArrayValueMutatingOperations() {
       // A store is only safe if it is to an array element and the element type
       // is trivial.
       if (auto *SI = dyn_cast<StoreInst>(Inst)) {
-        if (!isArrayEltStore(SI) ||
+        if (!isAddressOfArrayElement(SI->getDest()) ||
             !SI->getSrc()->getType().isTrivial(Module)) {
           LLVM_DEBUG(llvm::dbgs()
                      <<"     (NO) non trivial store could store an array value "
@@ -1420,7 +1406,7 @@ bool COWArrayOpt::hasLoopOnlyDestructorSafeArrayOperations() {
 
       // Stores to array elements.
       if (auto *SI = dyn_cast<StoreInst>(Inst)) {
-        if (isArrayEltStore(SI))
+        if (isAddressOfArrayElement(SI->getDest()))
           continue;
         LLVM_DEBUG(llvm::dbgs() << "     (NO) unknown store " << *SI);
         return ReturnWithCleanup(false);

lib/SILOptimizer/Utils/Local.cpp

@@ -329,6 +329,43 @@ bool swift::mayBindDynamicSelf(SILFunction *F) {
   return false;
 }
 
+static SILValue skipAddrProjections(SILValue V) {
+  for (;;) {
+    switch (V->getKind()) {
+      case ValueKind::IndexAddrInst:
+      case ValueKind::IndexRawPointerInst:
+      case ValueKind::StructElementAddrInst:
+      case ValueKind::TupleElementAddrInst:
+        V = cast<SingleValueInstruction>(V)->getOperand(0);
+        break;
+      default:
+        return V;
+    }
+  }
+  llvm_unreachable("there is no escape from an infinite loop");
+}
+
+/// Check whether the \p addr is an address of a tail-allocated array element.
+bool swift::isAddressOfArrayElement(SILValue addr) {
+  addr = stripAddressProjections(addr);
+  if (auto *MD = dyn_cast<MarkDependenceInst>(addr))
+    addr = stripAddressProjections(MD->getValue());
+
+  // High-level SIL: check for an get_element_address array semantics call.
+  if (auto *PtrToAddr = dyn_cast<PointerToAddressInst>(addr))
+    if (auto *SEI = dyn_cast<StructExtractInst>(PtrToAddr->getOperand())) {
+      ArraySemanticsCall Call(SEI->getOperand());
+      if (Call && Call.getKind() == ArrayCallKind::kGetElementAddress)
+        return true;
+    }
+
+  // Check for an tail-address (of an array buffer object).
+  if (isa<RefTailAddrInst>(skipAddrProjections(addr)))
+    return true;
+
+  return false;
+}
+
 /// Find a new position for an ApplyInst's FuncRef so that it dominates its
 /// use. Not that FunctionRefInsts may be shared by multiple ApplyInsts.
 void swift::placeFuncRef(ApplyInst *AI, DominanceInfo *DT) {

test/SILOptimizer/abcopts.sil

@@ -12,6 +12,10 @@ struct ArrayIntBuffer {
   var storage : Builtin.NativeObject
 }
 
+final class StorageBase {
+  @sil_stored var header: Int64
+}
+
 struct ArrayInt{
   var buffer : ArrayIntBuffer
 }
@@ -665,6 +669,68 @@ bb3:
   return %23 : $Int32
 }
 
+// HOIST-LABEL: sil @hoist_rangechecked_ref_tail_addr
+// HOIST: bb0
+// HOIST:  cond_br {{.*}}, bb1{{.*}}, bb2
+// HOIST: bb1:
+// HOIST: br bb6
+// HOIST: bb2:
+// HOIST:   [[CB:%[0-9]+]] = function_ref @checkbounds
+// HOIST:    apply [[CB]]
+// HOIST:   br bb3{{.*}}
+// HOIST: bb3{{.*}}:
+// HOIST-NOT: function_ref @checkbounds
+// HOIST-NOT:    apply [[CB]]
+// HOIST:   cond_br {{.*}}, bb5{{.*}}, bb4{{.*}}
+// HOIST: bb4
+// HOIST:   br bb3
+// HOIST: bb5
+// HOIST:   br bb6
+// HOIST: bb6{{.*}}:
+// HOIST:  return
+
+sil @hoist_rangechecked_ref_tail_addr : $@convention(thin) (Int32, @inout ArrayInt) -> Int32 {
+bb0(%0 : $Int32, %24 : $*ArrayInt):
+  %100 = integer_literal $Builtin.Int1, -1
+  %101 = struct $Bool(%100 : $Builtin.Int1)
+  %1 = struct_extract %0 : $Int32, #Int32._value
+  %2 = integer_literal $Builtin.Int32, 0
+  %3 = integer_literal $Builtin.Int1, -1
+  %61 = builtin "cmp_sle_Int32"(%2 : $Builtin.Int32, %1 : $Builtin.Int32) : $Builtin.Int1
+  %14 = builtin "xor_Int1"(%61 : $Builtin.Int1, %3 : $Builtin.Int1) : $Builtin.Int1
+  cond_fail %14 : $Builtin.Int1
+  br bb1(%2 : $Builtin.Int32)
+
+bb1(%4 : $Builtin.Int32):
+  %8 = builtin "cmp_eq_Int32"(%4 : $Builtin.Int32, %1 : $Builtin.Int32) : $Builtin.Int1
+  cond_br %8, bb3, bb4
+
+bb4:
+  %37 = struct $Int32(%4 : $Builtin.Int32)
+  %52 = function_ref @checkbounds : $@convention(method) (Int32, Bool, @owned ArrayInt) -> _DependenceToken
+  %53 = load %24 : $*ArrayInt
+  %54 = struct_extract %53 : $ArrayInt, #ArrayInt.buffer
+  %55 = struct_extract %54 : $ArrayIntBuffer, #ArrayIntBuffer.storage
+  retain_value %55 : $Builtin.NativeObject
+  %58 = apply %52(%37, %101, %53) : $@convention(method) (Int32, Bool, @owned ArrayInt) -> _DependenceToken
+  %10 = integer_literal $Builtin.Int32, 1
+  %19 = integer_literal $Builtin.Int1, -1
+  %20 = builtin "sadd_with_overflow_Int32"(%4 : $Builtin.Int32, %10 : $Builtin.Int32, %19 : $Builtin.Int1) : $(Builtin.Int32, Builtin.Int1)
+  %21 = tuple_extract %20 : $(Builtin.Int32, Builtin.Int1), 0
+
+  retain_value %55 : $Builtin.NativeObject
+
+  %69 = unchecked_ref_cast %55 : $Builtin.NativeObject to $StorageBase
+  %70 = ref_tail_addr %69 : $StorageBase, $Int32
+  %71 = index_addr %70 : $*Int32, %4 : $Builtin.Int32
+  store %0 to %71 : $*Int32
+  br bb1(%21 : $Builtin.Int32)
+
+bb3:
+  %23 = struct $Int32 (%4 : $Builtin.Int32)
+  return %23 : $Int32
+}
+
 // HOIST-LABEL: sil @eliminate_zero_to_count
 // HOIST-NOT: function_ref @checkbounds2
 // HOIST:  return