SILGen: improve code generation for array literals

It used to be done with a library intrinsic which returns the array and an element address pointer.
A `mark_dependence` was used to "connect" the two results.
But this resulted in completely wrong OSSA after inlining.
It just didn't get caught be the verifier because the SIL was obfuscated by address-pointer conversions.

Now we don't use the second result (= the element address) of the intrinsic but generate a correct borrow scope from the first (= array) result. Within that borrow scope we project out the element address with a `ref_tail_addr` instruction.

This relies on knowledge of the internal Array data structures. However those data structures are baked into the ABI since a long time and will not change.

Author: eeckstein

lib/SILGen/SILGenApply.cpp

@@ -7058,7 +7058,7 @@ RValue SILGenFunction::emitLiteral(LiteralExpr *literal, SGFContext C) {
 /// Allocate an uninitialized array of a given size, returning the array
 /// and a pointer to its uninitialized contents, which must be initialized
 /// before the array is valid.
-std::pair<ManagedValue, SILValue>
+ManagedValue
 SILGenFunction::emitUninitializedArrayAllocation(Type ArrayTy,
                                                  SILValue Length,
                                                  SILLocation Loc) {
@@ -7075,11 +7075,13 @@ SILGenFunction::emitUninitializedArrayAllocation(Type ArrayTy,
   SmallVector<ManagedValue, 2> resultElts;
   std::move(result).getAll(resultElts);
 
-  // Add a mark_dependence between the interior pointer and the array value
-  auto dependentValue = B.createMarkDependence(Loc, resultElts[1].getValue(),
-                                               resultElts[0].getValue(),
-                                               MarkDependenceKind::Escaping);
-  return {resultElts[0], dependentValue};
+  // The second result, which is the base element address, is not used. We extract
+  // it from the array (= the first result) directly to create a correct borrow scope.
+  // TODO: Consider adding a new intrinsic which only returns the array.
+  // Although the current intrinsic is inlined and the code for returning the
+  // second result is optimized away. So it doesn't make a performance difference.
+
+  return resultElts[0];
 }
 
 /// Deallocate an uninitialized array.

lib/SILGen/SILGenExpr.cpp

@@ -2664,6 +2664,29 @@ RValue RValueEmitter::visitUnreachableExpr(UnreachableExpr *E, SGFContext C) {
   return RValue(SGF, E, ManagedValue::forRValueWithoutOwnership(undef));
 }
 
+static SILValue getArrayBuffer(SILValue array, SILGenFunction &SGF, SILLocation loc) {
+  SILValue v = array;
+  SILType storageType;
+  while (auto *sd = v->getType().getStructOrBoundGenericStruct()) {
+    ASSERT(sd->getStoredProperties().size() == 1 &&
+           "Array or its internal structs should have exactly one stored property");
+    auto *se = SGF.getBuilder().createStructExtract(loc, v, v->getType().getFieldDecl(0));
+    if (se->getType() == SILType::getBridgeObjectType(SGF.getASTContext())) {
+      auto bridgeObjTy = cast<BoundGenericStructType>(v->getType().getASTType());
+      CanType ct = CanType(bridgeObjTy->getGenericArgs()[0]);
+      storageType = SILType::getPrimitiveObjectType(ct);
+    }
+    v = se;
+  }
+
+  if (storageType) {
+    v = SGF.getBuilder().createUncheckedRefCast(loc, v, storageType);
+  }
+  ASSERT(v->getType().isReferenceCounted(&SGF.F) &&
+         "expected a reference-counted buffer in the Array data type");
+  return v;
+}
+
 VarargsInfo Lowering::emitBeginVarargs(SILGenFunction &SGF, SILLocation loc,
                                        CanType baseTy, CanType arrayTy,
                                        unsigned numElements) {
@@ -2675,12 +2698,7 @@ VarargsInfo Lowering::emitBeginVarargs(SILGenFunction &SGF, SILLocation loc,
   SILValue numEltsVal = SGF.B.createIntegerLiteral(loc,
                              SILType::getBuiltinWordType(SGF.getASTContext()),
                              numElements);
-  // The first result is the array value.
-  ManagedValue array;
-  // The second result is a RawPointer to the base address of the array.
-  SILValue basePtr;
-  std::tie(array, basePtr)
-    = SGF.emitUninitializedArrayAllocation(arrayTy, numEltsVal, loc);
+  ManagedValue array = SGF.emitUninitializedArrayAllocation(arrayTy, numEltsVal, loc);
 
   // Temporarily deactivate the main array cleanup.
   if (array.hasCleanup())
@@ -2690,13 +2708,15 @@ VarargsInfo Lowering::emitBeginVarargs(SILGenFunction &SGF, SILLocation loc,
   auto abortCleanup =
     SGF.enterDeallocateUninitializedArrayCleanup(array.getValue());
 
-  // Turn the pointer into an address.
-  basePtr = SGF.B.createPointerToAddress(
-    loc, basePtr, baseTL.getLoweredType().getAddressType(),
-    /*isStrict*/ true,
-    /*isInvariant*/ false);
+  auto borrowedArray = array.borrow(SGF, loc);
+  auto borrowCleanup = SGF.Cleanups.getTopCleanup();
+
+  SILValue buffer = getArrayBuffer(borrowedArray.getValue(), SGF, loc);
 
-  return VarargsInfo(array, abortCleanup, basePtr, baseTL, baseAbstraction);
+  SILType elementAddrTy = baseTL.getLoweredType().getAddressType();
+  SILValue baseAddr = SGF.getBuilder().createRefTailAddr(loc, buffer, elementAddrTy);
+
+  return VarargsInfo(array, borrowCleanup, abortCleanup, baseAddr, baseTL, baseAbstraction);
 }
 
 ManagedValue Lowering::emitEndVarargs(SILGenFunction &SGF, SILLocation loc,
@@ -2710,6 +2730,8 @@ ManagedValue Lowering::emitEndVarargs(SILGenFunction &SGF, SILLocation loc,
   if (array.hasCleanup())
     SGF.Cleanups.setCleanupState(array.getCleanup(), CleanupState::Active);
 
+  SGF.Cleanups.popAndEmitCleanup(varargs.getBorrowCleanup(), CleanupLocation(loc), NotForUnwind);
+
   // Array literals only need to be finalized, if the array is really allocated.
   // In case of zero elements, no allocation is done, but the empty-array
   // singleton is used. "Finalization" means to emit an end_cow_mutation

lib/SILGen/SILGenFunction.h

@@ -1899,11 +1899,10 @@ class LLVM_LIBRARY_VISIBILITY SILGenFunction
   ManagedValue emitUndef(Type type);
   ManagedValue emitUndef(SILType type);
   RValue emitUndefRValue(SILLocation loc, Type type);
-  
-  std::pair<ManagedValue, SILValue>
-  emitUninitializedArrayAllocation(Type ArrayTy,
-                                   SILValue Length,
-                                   SILLocation Loc);
+
+  ManagedValue emitUninitializedArrayAllocation(Type ArrayTy,
+                                                SILValue Length,
+                                                SILLocation Loc);
 
   CleanupHandle enterDeallocateUninitializedArrayCleanup(SILValue array);
   void emitUninitializedArrayDeallocation(SILLocation loc, SILValue array);

lib/SILGen/Varargs.h

@@ -31,22 +31,26 @@ class TypeLowering;
 /// Information about a varargs emission.
 class VarargsInfo {
   ManagedValue Array;
+  CleanupHandle borrowCleanup;
   CleanupHandle AbortCleanup;
   SILValue BaseAddress;
   AbstractionPattern BasePattern;
   const TypeLowering &BaseTL;
 public:
-  VarargsInfo(ManagedValue array, CleanupHandle abortCleanup,
+  VarargsInfo(ManagedValue array, CleanupHandle borrowCleanup, CleanupHandle abortCleanup,
               SILValue baseAddress, const TypeLowering &baseTL,
               AbstractionPattern basePattern)
-    : Array(array), AbortCleanup(abortCleanup),
+    : Array(array), borrowCleanup(borrowCleanup), AbortCleanup(abortCleanup),
       BaseAddress(baseAddress), BasePattern(basePattern), BaseTL(baseTL) {}
 
   /// Return the array value.  emitEndVarargs() is really the only
   /// function that should be accessing this directly.
   ManagedValue getArray() const {
     return Array;
   }
+
+  CleanupHandle getBorrowCleanup() const { return borrowCleanup; }
+
   CleanupHandle getAbortCleanup() const { return AbortCleanup; }
 
   /// An address of the lowered type.

test/IRGen/unmanaged_objc_throw_func.swift

@@ -15,7 +15,8 @@ import Foundation
   func returnUnmanagedCFArray() throws -> Unmanaged<CFArray> {
     // CHECK: %[[T0:.+]] = call swiftcc { ptr, ptr } @"$ss27_allocateUninitializedArrayySayxG_BptBwlF"(i{{32|64}} 1, ptr @"$sSiN")
     // CHECK-NEXT: %[[T1:.+]] = extractvalue { ptr, ptr } %[[T0]], 0
-    // CHECK-NEXT: %[[T2:.+]] = extractvalue { ptr, ptr } %[[T0]], 1
+    // CHECK-NEXT:            = extractvalue { ptr, ptr } %[[T0]], 1
+    // CHECK-NEXT: %[[T2:.+]] = getelementptr inbounds i8, ptr %[[T1]]
     // CHECK-NEXT: %._value = getelementptr inbounds{{.*}} %TSi, ptr %[[T2]], i32 0, i32 0
     // CHECK:      %[[T7:.+]] = call swiftcc ptr @"$ss27_finalizeUninitializedArrayySayxGABnlF"(ptr %[[T1]], ptr @"$sSiN")
     // CHECK:      %[[T4:.+]] = call swiftcc ptr @"$sSa10FoundationE19_bridgeToObjectiveCSo7NSArrayCyF"(ptr %[[T7]], ptr @"$sSiN")

test/Interop/Cxx/foreign-reference/reference-counted-irgen.swift

@@ -74,6 +74,6 @@ public func getArrayOfLocalCount() -> [NS.LocalCount] {
 // CHECK-NEXT:   %0 = call swiftcc %swift.metadata_response @"$sSo2NSO10LocalCountVMa"(i{{.*}} 0)
 // CHECK-NEXT:   %1 = extractvalue %swift.metadata_response %0, 0
 // CHECK-NEXT:   %2 = call swiftcc { ptr, ptr } @"$ss27_allocateUninitializedArrayySayxG_BptBwlF"(i{{.*}} 1, ptr %1)
-// CHECK:        %5 = call ptr @{{_ZN2NS10LocalCount6createEv|"\?create\@LocalCount\@NS\@\@SAPEAU12\@XZ"}}()
-// CHECK-NEXT:   call void @{{_Z8LCRetainPN2NS10LocalCountE|"\?LCRetain\@\@YAXPEAULocalCount\@NS\@\@\@Z"}}(ptr %5)
+// CHECK:        %6 = call ptr @{{_ZN2NS10LocalCount6createEv|"\?create\@LocalCount\@NS\@\@SAPEAU12\@XZ"}}()
+// CHECK-NEXT:   call void @{{_Z8LCRetainPN2NS10LocalCountE|"\?LCRetain\@\@YAXPEAULocalCount\@NS\@\@\@Z"}}(ptr %6)
 // CHECK:      }

test/SILGen/arguments.swift

@@ -6,15 +6,19 @@ struct UnicodeScalar {}
 
 enum Never {}
 
+class ArrayBufer {}
+
 // Minimal implementation to support varargs.
-struct Array<T> { }
+struct Array<T> {
+  let buffer: ArrayBufer
+}
 
 func _allocateUninitializedArray<T>(_: Builtin.Word)
   -> (Array<T>, Builtin.RawPointer) {
   Builtin.int_trap()
 }
 
-func _finalizeUninitializedArray<T>(_ a: Array<T>) -> Array<T> {
+func _finalizeUninitializedArray<T>(_ a: __owned Array<T>) -> Array<T> {
   return a
 }
 
@@ -63,7 +67,7 @@ arg_default_tuple(x:i, y:f)
 
 func variadic_arg_1(_ x: Int...) {}
 // CHECK-LABEL: sil hidden [ossa] @$ss14variadic_arg_1{{[_0-9a-zA-Z]*}}F
-// CHECK: bb0([[X:%[0-9]+]] : $Array<Int>):
+// CHECK: bb0([[X:%[0-9]+]] : @guaranteed $Array<Int>):
 
 variadic_arg_1()
 variadic_arg_1(i)
@@ -72,23 +76,23 @@ variadic_arg_1(i, i, i)
 
 func variadic_arg_2(_ x: Int, _ y: Float...) {}
 // CHECK-LABEL: sil hidden [ossa] @$ss14variadic_arg_2{{[_0-9a-zA-Z]*}}F
-// CHECK: bb0([[X:%[0-9]+]] : $Int, [[Y:%[0-9]+]] : $Array<Float>):
+// CHECK: bb0([[X:%[0-9]+]] : $Int, [[Y:%[0-9]+]] : @guaranteed $Array<Float>):
 
 variadic_arg_2(i)
 variadic_arg_2(i, f)
 variadic_arg_2(i, f, f, f)
 
 func variadic_arg_3(_ y: Float..., x: Int) {}
 // CHECK-LABEL: sil hidden [ossa] @$ss14variadic_arg_3{{[_0-9a-zA-Z]*}}F
-// CHECK: bb0([[Y:%[0-9]+]] : $Array<Float>, [[X:%[0-9]+]] : $Int):
+// CHECK: bb0([[Y:%[0-9]+]] : @guaranteed $Array<Float>, [[X:%[0-9]+]] : $Int):
 
 func variadic_arg_4(_ y: Float..., x: Int...) {}
 // CHECK-LABEL: sil hidden [ossa] @$ss14variadic_arg_4{{[_0-9a-zA-Z]*}}F
-// CHECK: bb0([[Y:%[0-9]+]] : $Array<Float>, [[X:%[0-9]+]] : $Array<Int>):
+// CHECK: bb0([[Y:%[0-9]+]] : @guaranteed $Array<Float>, [[X:%[0-9]+]] : @guaranteed $Array<Int>):
 
 func variadic_arg_5(a: Int, b: Float..., c: Int, d: Int...) {}
 // CHECK-LABEL: sil hidden [ossa] @$ss14variadic_arg_5{{[_0-9a-zA-Z]*}}F
-// CHECK: bb0([[A:%[0-9]+]] : $Int, [[B:%[0-9]+]] : $Array<Float>, [[C:%[0-9]+]] : $Int, [[D:%[0-9]+]] : $Array<Int>):
+// CHECK: bb0([[A:%[0-9]+]] : $Int, [[B:%[0-9]+]] : @guaranteed $Array<Float>, [[C:%[0-9]+]] : $Int, [[D:%[0-9]+]] : @guaranteed $Array<Int>):
 
 variadic_arg_3(x: i)
 variadic_arg_3(f, x: i)

test/SILGen/array_literal_abstraction.swift

@@ -5,21 +5,21 @@
 // <rdar://problem/16039286>
 
 // CHECK-LABEL: sil hidden [ossa] @$s25array_literal_abstraction0A9_of_funcsSayyycGyF
-// CHECK:         pointer_to_address {{.*}} $*@callee_guaranteed @substituted <τ_0_0> () -> @out τ_0_0 for <()>
+// CHECK:         ref_tail_addr %{{[0-9]+}}, $@callee_guaranteed @substituted <τ_0_0> () -> @out τ_0_0 for <()>
 func array_of_funcs() -> [(() -> ())] {
   return [{}, {}]
 }
 
 // CHECK-LABEL: sil hidden [ossa] @$s25array_literal_abstraction13dict_of_funcsSDySiyycGyF
-// CHECK:         pointer_to_address {{.*}} $*(Int, @callee_guaranteed @substituted <τ_0_0> () -> @out τ_0_0 for <()>)
+// CHECK:         ref_tail_addr %{{[0-9]+}}, $(Int, @callee_guaranteed @substituted <τ_0_0> () -> @out τ_0_0 for <()>)
 func dict_of_funcs() -> Dictionary<Int, () -> ()> {
   return [0: {}, 1: {}]
 }
 
 func vararg_funcs(_ fs: (() -> ())...) {}
 
 // CHECK-LABEL: sil hidden [ossa] @$s25array_literal_abstraction17call_vararg_funcsyyF
-// CHECK:         pointer_to_address {{.*}} $*@callee_guaranteed @substituted <τ_0_0> () -> @out τ_0_0 for <()>
+// CHECK:         ref_tail_addr %{{[0-9]+}}, $@callee_guaranteed @substituted <τ_0_0> () -> @out τ_0_0 for <()>
 func call_vararg_funcs() {
   vararg_funcs({}, {})
 }

test/SILGen/errors.swift

@@ -615,8 +615,9 @@ func test_variadic(_ cat: Cat) throws {
 // CHECK:         [[T0:%.*]] = function_ref @$ss27_allocateUninitializedArray{{.*}}F
 // CHECK:         [[T1:%.*]] = apply [[T0]]<Cat>([[N]])
 // CHECK:         ([[ARRAY:%.*]], [[T2:%.*]]) = destructure_tuple [[T1]]
-// CHECK:         [[MDI:%.*]] = mark_dependence [[T2]]  : $Builtin.RawPointer on [[ARRAY]]
-// CHECK:         [[ELT0:%.*]] = pointer_to_address [[MDI]] : $Builtin.RawPointer to [strict] $*Cat
+// CHECK:         [[BB:%.*]] = begin_borrow [[ARRAY]]
+// CHECK:            struct_extract [[BB]]
+// CHECK:         [[ELT0:%.*]] = ref_tail_addr
 //   Element 0.
 // CHECK:         [[T0:%.*]] = function_ref @$s6errors10make_a_catAA3CatCyKF : $@convention(thin) () -> (@owned Cat, @error any Error)
 // CHECK:         try_apply [[T0]]() : $@convention(thin) () -> (@owned Cat, @error any Error), normal [[NORM_0:bb[0-9]+]], error [[ERR_0:bb[0-9]+]]
@@ -654,7 +655,7 @@ func test_variadic(_ cat: Cat) throws {
 // CHECK-NEXT:    return
 //   Failure from element 0.
 // CHECK:       [[ERR_0]]([[ERROR:%.*]] : @owned $any Error):
-// CHECK-NOT:     end_borrow
+// CHECK:         end_borrow [[BB]]
 // CHECK-NEXT:    // function_ref
 // CHECK-NEXT:    [[T0:%.*]] = function_ref @$ss29_deallocateUninitializedArray{{.*}}F
 // CHECK-NEXT:    apply [[T0]]<Cat>([[ARRAY]])
@@ -663,6 +664,7 @@ func test_variadic(_ cat: Cat) throws {
 // CHECK:       [[ERR_2]]([[ERROR:%.*]] : @owned $any Error):
 // CHECK-NEXT:    destroy_addr [[ELT1]]
 // CHECK-NEXT:    destroy_addr [[ELT0]]
+// CHECK:         end_borrow [[BB]]
 // CHECK-NEXT:    // function_ref
 // CHECK-NEXT:    [[T0:%.*]] = function_ref @$ss29_deallocateUninitializedArray{{.*}}F
 // CHECK-NEXT:    apply [[T0]]<Cat>([[ARRAY]])
@@ -672,6 +674,7 @@ func test_variadic(_ cat: Cat) throws {
 // CHECK-NEXT:    destroy_addr [[ELT2]]
 // CHECK-NEXT:    destroy_addr [[ELT1]]
 // CHECK-NEXT:    destroy_addr [[ELT0]]
+// CHECK-NEXT:    end_borrow [[BB]]
 // CHECK-NEXT:    // function_ref
 // CHECK-NEXT:    [[T0:%.*]] = function_ref @$ss29_deallocateUninitializedArray{{.*}}F
 // CHECK-NEXT:    apply [[T0]]<Cat>([[ARRAY]])