Merge pull request #78599 from atrick/fix-diagnostic-hack

LifetimeDependenceDiagnostics; remove a bootstrapping hack.

Author: atrick

SwiftCompilerSources/Sources/Optimizer/FunctionPasses/LifetimeDependenceDiagnostics.swift

@@ -199,21 +199,6 @@ private struct DiagnoseDependence {
     if function.hasUnsafeNonEscapableResult {
       return .continueWalk
     }
-    // FIXME: remove this condition once we have a Builtin.dependence,
-    // which developers should use to model the unsafe
-    // dependence. Builtin.lifetime_dependence will be lowered to
-    // mark_dependence [unresolved], which will be checked
-    // independently. Instead, of this function result check, allow
-    // isUnsafeApplyResult to be used be mark_dependence [unresolved]
-    // without checking its dependents.
-    //
-    // Allow returning an apply result (@_unsafeNonescapableResult) if
-    // the calling function has a dependence. This implicitly makes
-    // the unsafe nonescapable result dependent on the calling
-    // function's lifetime dependence arguments.
-    if dependence.isUnsafeApplyResult, function.hasResultDependence {
-      return .continueWalk
-    }
     // Check that the parameter dependence for this result is the same
     // as the current dependence scope.
     if let arg = dependence.scope.parentValue as? FunctionArgument,

SwiftCompilerSources/Sources/Optimizer/Utilities/LifetimeDependenceUtils.swift

@@ -225,16 +225,6 @@ extension LifetimeDependence {
     self.dependentValue = value
   }
 
-  var isUnsafeApplyResult: Bool {
-    if case let .owned(value) = scope {
-      if let apply = value.definingInstruction as? FullApplySite {
-        assert(!apply.hasResultDependence)
-        return true
-      }
-    }
-    return false
-  }
-
   /// Construct LifetimeDependence from mark_dependence [unresolved] or mark_dependence [nonescaping].
   ///
   /// For any LifetimeDependence constructed from a mark_dependence, its `dependentValue` will be the result of the

lib/SILOptimizer/Mandatory/PredictableMemOpt.cpp

@@ -91,23 +91,28 @@ getFullyReferenceableStruct(SILType Ty) {
   return SD;
 }
 
-static unsigned getNumSubElements(SILType T, SILModule &M,
+static unsigned getNumSubElements(SILType T, SILFunction &F,
                                   TypeExpansionContext context) {
 
   if (auto TT = T.getAs<TupleType>()) {
     unsigned NumElements = 0;
     for (auto index : indices(TT.getElementTypes()))
       NumElements +=
-          getNumSubElements(T.getTupleElementType(index), M, context);
+          getNumSubElements(T.getTupleElementType(index), F, context);
     return NumElements;
   }
 
   if (auto *SD = getFullyReferenceableStruct(T)) {
     unsigned NumElements = 0;
     for (auto *D : SD->getStoredProperties())
       NumElements +=
-          getNumSubElements(T.getFieldType(D, M, context), M, context);
-    return NumElements;
+        getNumSubElements(T.getFieldType(D, F.getModule(), context), F,
+                          context);
+
+    // Returning NumElements == 0 implies that "empty" values can be
+    // materialized without ownership. This is only valid for trivial types.
+    if (NumElements > 0 || T.isTrivial(F))
+      return NumElements;
   }
 
   // If this isn't a tuple or struct, it is a single element.
@@ -152,7 +157,7 @@ static SILValue getAccessPathRoot(SILValue pointer) {
 static unsigned computeSubelement(SILValue Pointer,
                                   SingleValueInstruction *RootInst) {
   unsigned SubElementNumber = 0;
-  SILModule &M = RootInst->getModule();
+  auto &F = *RootInst->getFunction();
 
   while (1) {
     // If we got to the root, we're done.
@@ -175,7 +180,7 @@ static unsigned computeSubelement(SILValue Pointer,
       // Keep track of what subelement is being referenced.
       for (unsigned i = 0, e = TEAI->getFieldIndex(); i != e; ++i) {
         SubElementNumber +=
-            getNumSubElements(TT.getTupleElementType(i), M,
+            getNumSubElements(TT.getTupleElementType(i), F,
                               TypeExpansionContext(*RootInst->getFunction()));
       }
       Pointer = TEAI->getOperand();
@@ -191,7 +196,8 @@ static unsigned computeSubelement(SILValue Pointer,
         if (D == SEAI->getField()) break;
         auto context = TypeExpansionContext(*RootInst->getFunction());
         SubElementNumber +=
-            getNumSubElements(ST.getFieldType(D, M, context), M, context);
+          getNumSubElements(ST.getFieldType(D, F.getModule(), context), F,
+                            context);
       }
 
       Pointer = SEAI->getOperand();
@@ -370,10 +376,9 @@ static bool isFullyAvailable(SILType loadTy, unsigned firstElt,
   if (!firstVal || firstVal.getType() != loadTy)
     return false;
 
-  auto *function = firstVal.getValue()->getFunction();
+  auto &function = *firstVal.getValue()->getFunction();
   return llvm::all_of(
-    range(getNumSubElements(loadTy, function->getModule(),
-                            TypeExpansionContext(*function))),
+    range(getNumSubElements(loadTy, function, TypeExpansionContext(function))),
     [&](unsigned index) -> bool {
       auto &val = AvailableValues[firstElt + index];
       return val.getValue() == firstVal.getValue() &&
@@ -395,7 +400,7 @@ static SILValue nonDestructivelyExtractSubElement(const AvailableValue &Val,
       // Keep track of what subelement is being referenced.
       SILType EltTy = ValTy.getTupleElementType(EltNo);
       unsigned NumSubElt = getNumSubElements(
-          EltTy, B.getModule(), TypeExpansionContext(B.getFunction()));
+          EltTy, B.getFunction(), TypeExpansionContext(B.getFunction()));
       if (SubElementNumber < NumSubElt) {
         auto BorrowedVal = Val.emitBeginBorrow(B, Loc);
         auto NewVal =
@@ -420,7 +425,7 @@ static SILValue nonDestructivelyExtractSubElement(const AvailableValue &Val,
       auto fieldType = ValTy.getFieldType(
           D, B.getModule(), TypeExpansionContext(B.getFunction()));
       unsigned NumSubElt = getNumSubElements(
-          fieldType, B.getModule(), TypeExpansionContext(B.getFunction()));
+          fieldType, B.getFunction(), TypeExpansionContext(B.getFunction()));
 
       if (SubElementNumber < NumSubElt) {
         auto BorrowedVal = Val.emitBeginBorrow(B, Loc);
@@ -1227,7 +1232,8 @@ bool AvailableValueAggregator::canTake(SILType loadTy,
     return llvm::all_of(indices(tt->getElements()), [&](unsigned eltNo) {
       SILType eltTy = loadTy.getTupleElementType(eltNo);
       unsigned numSubElt =
-          getNumSubElements(eltTy, M, TypeExpansionContext(B.getFunction()));
+        getNumSubElements(eltTy, B.getFunction(),
+                          TypeExpansionContext(B.getFunction()));
       bool success = canTake(eltTy, firstElt);
       firstElt += numSubElt;
       return success;
@@ -1238,7 +1244,7 @@ bool AvailableValueAggregator::canTake(SILType loadTy,
     return llvm::all_of(sd->getStoredProperties(), [&](VarDecl *decl) -> bool {
       auto context = TypeExpansionContext(B.getFunction());
       SILType eltTy = loadTy.getFieldType(decl, M, context);
-      unsigned numSubElt = getNumSubElements(eltTy, M, context);
+      unsigned numSubElt = getNumSubElements(eltTy, B.getFunction(), context);
       bool success = canTake(eltTy, firstElt);
       firstElt += numSubElt;
       return success;
@@ -1369,7 +1375,8 @@ SILValue AvailableValueAggregator::aggregateTupleSubElts(TupleType *TT,
   for (unsigned EltNo : indices(TT->getElements())) {
     SILType EltTy = LoadTy.getTupleElementType(EltNo);
     unsigned NumSubElt =
-        getNumSubElements(EltTy, M, TypeExpansionContext(B.getFunction()));
+      getNumSubElements(EltTy, B.getFunction(),
+                        TypeExpansionContext(B.getFunction()));
 
     // If we are missing any of the available values in this struct element,
     // compute an address to load from.
@@ -1406,7 +1413,7 @@ SILValue AvailableValueAggregator::aggregateStructSubElts(StructDecl *sd,
   for (auto *decl : sd->getStoredProperties()) {
     auto context = TypeExpansionContext(B.getFunction());
     SILType eltTy = loadTy.getFieldType(decl, M, context);
-    unsigned numSubElt = getNumSubElements(eltTy, M, context);
+    unsigned numSubElt = getNumSubElements(eltTy, B.getFunction(), context);
 
     // If we are missing any of the available values in this struct element,
     // compute an address to load from.
@@ -1540,6 +1547,8 @@ class AvailableValueDataflowContext {
 private:
   SILModule &getModule() const { return TheMemory->getModule(); }
 
+  SILFunction &getFunction() const { return *TheMemory->getFunction(); }
+
   void updateAvailableValues(
     SILInstruction *Inst,
     SmallBitVector &RequiredElts,
@@ -1673,7 +1682,7 @@ AvailableValueDataflowContext::computeAvailableValues(
     return std::nullopt;
 
   unsigned NumLoadSubElements = getNumSubElements(
-    LoadTy, getModule(), TypeExpansionContext(*TheMemory->getFunction()));
+    LoadTy, getFunction(), TypeExpansionContext(getFunction()));
 
   LoadInfo loadInfo = {LoadTy, FirstElt, NumLoadSubElements};
 
@@ -1712,14 +1721,14 @@ static inline void updateAvailableValuesHelper(
     SmallBitVector &conflictingValues,
     function_ref<std::optional<AvailableValue>(unsigned)> defaultFunc,
     function_ref<bool(AvailableValue &, unsigned)> isSafeFunc) {
-  auto &mod = theMemory->getModule();
   unsigned startSubElt = computeSubelement(address, theMemory);
 
   // TODO: Is this needed now?
   assert(startSubElt != ~0U && "Store within enum projection not handled");
+  auto &f = *theMemory->getFunction();
   for (unsigned i : range(getNumSubElements(
-           address->getType().getObjectType(), mod,
-           TypeExpansionContext(*theMemory->getFunction())))) {
+                            address->getType().getObjectType(), f,
+                            TypeExpansionContext(f)))) {
     // If this element is not required, don't fill it in.
     if (!requiredElts[startSubElt + i])
       continue;
@@ -1849,7 +1858,8 @@ void AvailableValueDataflowContext::updateAvailableValues(
 
     bool AnyRequired = false;
     for (unsigned i : range(getNumSubElements(
-             ValTy, getModule(), TypeExpansionContext(*CAI->getFunction())))) {
+                              ValTy, getFunction(),
+                              TypeExpansionContext(getFunction())))) {
       // If this element is not required, don't fill it in.
       AnyRequired = RequiredElts[StartSubElt+i];
       if (AnyRequired) break;
@@ -1883,7 +1893,7 @@ void AvailableValueDataflowContext::updateAvailableValues(
     // potentially bailing out (because it is address-only).
     bool AnyRequired = false;
     for (unsigned i : range(getNumSubElements(
-             ValTy, getModule(), TypeExpansionContext(*MD->getFunction())))) {
+             ValTy, getFunction(), TypeExpansionContext(getFunction())))) {
       // If this element is not required, don't fill it in.
       AnyRequired = RequiredElts[StartSubElt+i];
       if (AnyRequired) break;
@@ -2171,7 +2181,7 @@ void AvailableValueDataflowContext::updateMarkDependenceValues(
   }
   SILType valueTy = md->getValue()->getType().getObjectType();
   unsigned numMDSubElements = getNumSubElements(
-    valueTy, getModule(), TypeExpansionContext(*TheMemory->getFunction()));
+    valueTy, getFunction(), TypeExpansionContext(getFunction()));
 
   // Update each required subelement of the mark_dependence value.
   for (unsigned subIdx = firstMDElt; subIdx < firstMDElt + numMDSubElements;
@@ -2299,7 +2309,8 @@ class OptimizeAllocLoads {
       : Module(memory->getModule()), TheMemory(memory),
         MemoryType(getMemoryType(memory)),
         NumMemorySubElements(getNumSubElements(
-            MemoryType, Module, TypeExpansionContext(*memory->getFunction()))),
+            MemoryType, *memory->getFunction(),
+            TypeExpansionContext(*memory->getFunction()))),
         Uses(uses), deleter(deleter), deadEndBlocks(deadEndBlocks),
         DataflowContext(TheMemory, NumMemorySubElements,
                         OptimizationMode::PreserveAlloc, uses,
@@ -2680,7 +2691,8 @@ class OptimizeDeadAlloc {
       : Module(memory->getModule()), TheMemory(memory),
         MemoryType(getMemoryType(memory)),
         NumMemorySubElements(getNumSubElements(
-            MemoryType, Module, TypeExpansionContext(*memory->getFunction()))),
+            MemoryType, *memory->getFunction(),
+            TypeExpansionContext(*memory->getFunction()))),
         Uses(uses), Releases(releases), deadEndBlocks(deadEndBlocks),
         deleter(deleter), domInfo(domInfo),
         DataflowContext(TheMemory, NumMemorySubElements,

test/SIL/explicit_lifetime_dependence_specifiers.swift

@@ -8,6 +8,30 @@
 
 import Builtin
 
+@_unsafeNonescapableResult
+@lifetime(borrow source)
+internal func _overrideLifetime<
+  T: ~Copyable & ~Escapable, U: ~Copyable & ~Escapable
+>(
+  _ dependent: consuming T, borrowing source: borrowing U
+) -> T {
+  // TODO: Remove @_unsafeNonescapableResult. Instead, the unsafe dependence
+  // should be expressed by a builtin that is hidden within the function body.
+  dependent
+}
+
+@_unsafeNonescapableResult
+@lifetime(source)
+internal func _overrideLifetime<
+  T: ~Copyable & ~Escapable, U: ~Copyable & ~Escapable
+>(
+  _ dependent: consuming T, copying source: borrowing U
+) -> T {
+  // TODO: Remove @_unsafeNonescapableResult. Instead, the unsafe dependence
+  // should be expressed by a builtin that is hidden within the function body.
+  dependent
+}
+
 struct BufferView : ~Escapable {
   let ptr: UnsafeRawBufferPointer
 // CHECK-LABEL: sil hidden @$s39explicit_lifetime_dependence_specifiers10BufferViewVyACSWcfC : $@convention(method) (UnsafeRawBufferPointer, @thin BufferView.Type) -> @lifetime(borrow 0)  @owned BufferView {
@@ -22,7 +46,7 @@ struct BufferView : ~Escapable {
     } 
     self.ptr = ptr
   }
-  @_unsafeNonescapableResult
+  @lifetime(borrow ptr)
   init(independent ptr: UnsafeRawBufferPointer) {
     self.ptr = ptr
   }
@@ -71,7 +95,8 @@ func derive(_ x: borrowing BufferView) -> BufferView {
 // CHECK-LABEL: sil hidden @$s39explicit_lifetime_dependence_specifiers16consumeAndCreateyAA10BufferViewVADnF : $@convention(thin) (@owned BufferView) -> @lifetime(copy 0)  @owned BufferView {
 @lifetime(x)
 func consumeAndCreate(_ x: consuming BufferView) -> BufferView {
-  return BufferView(independent: x.ptr)
+  let bv = BufferView(independent: x.ptr)
+  return _overrideLifetime(bv, copying: x)
 }
 
 // CHECK-LABEL: sil hidden @$s39explicit_lifetime_dependence_specifiers17deriveThisOrThat1yAA10BufferViewVAD_ADtF : $@convention(thin) (@guaranteed BufferView, @guaranteed BufferView) -> @lifetime(copy 1, borrow 0)  @owned BufferView {
@@ -89,7 +114,8 @@ func deriveThisOrThat2(_ this: borrowing BufferView, _ that: consuming BufferVie
   if (Int.random(in: 1..<100) == 0) {
     return BufferView(independent: this.ptr)
   }
-  return BufferView(independent: that.ptr)
+  let bv = BufferView(independent: that.ptr)
+  return _overrideLifetime(bv, copying: that)
 }
 
 func use(_ x: borrowing BufferView) {}
@@ -123,12 +149,12 @@ struct Container : ~Escapable {
 // CHECK-LABEL: sil hidden @$s39explicit_lifetime_dependence_specifiers16getConsumingViewyAA06BufferG0VAA9ContainerVnF : $@convention(thin) (@owned Container) -> @lifetime(copy 0)  @owned BufferView {
 @lifetime(x)
 func getConsumingView(_ x: consuming Container) -> BufferView {
-  return BufferView(independent: x.ptr)
+  let bv = BufferView(independent: x.ptr)
+  return _overrideLifetime(bv, copying: x)
 }
 
 // CHECK-LABEL: sil hidden @$s39explicit_lifetime_dependence_specifiers16getBorrowingViewyAA06BufferG0VAA9ContainerVF : $@convention(thin) (@guaranteed Container) -> @lifetime(borrow 0)  @owned BufferView {
 @lifetime(borrow x)
 func getBorrowingView(_ x: borrowing Container) -> BufferView {
   return BufferView(independent: x.ptr)
 }
-

test/SIL/implicit_lifetime_dependence.swift

@@ -5,11 +5,27 @@
 
 // REQUIRES: swift_feature_LifetimeDependence
 
+@_unsafeNonescapableResult
+@lifetime(borrow source)
+internal func _overrideLifetime<
+  T: ~Copyable & ~Escapable, U: ~Copyable & ~Escapable
+>(
+  _ dependent: consuming T, borrowing source: borrowing U
+) -> T {
+  // TODO: Remove @_unsafeNonescapableResult. Instead, the unsafe dependence
+  // should be expressed by a builtin that is hidden within the function body.
+  dependent
+}
+
 @_unsafeNonescapableResult
 @lifetime(source)
-func unsafeLifetime<T: ~Copyable & ~Escapable, U: ~Copyable & ~Escapable>(
-  dependent: consuming T, dependsOn source: borrowing U)
-  -> T {
+internal func _overrideLifetime<
+  T: ~Copyable & ~Escapable, U: ~Copyable & ~Escapable
+>(
+  _ dependent: consuming T, copying source: borrowing U
+) -> T {
+  // TODO: Remove @_unsafeNonescapableResult. Instead, the unsafe dependence
+  // should be expressed by a builtin that is hidden within the function body.
   dependent
 }
 
@@ -21,7 +37,7 @@ struct BufferView : ~Escapable {
     self.ptr = ptr
     self.c = c
   }
-  @_unsafeNonescapableResult
+  @lifetime(borrow ptr)
   init(independent ptr: UnsafeRawBufferPointer, _ c: Int) {
     self.ptr = ptr
     self.c = c
@@ -74,7 +90,8 @@ func derive(_ unused: Int, _ x: borrowing BufferView) -> BufferView {
 
 // CHECK-LABEL: sil hidden @$s28implicit_lifetime_dependence16consumeAndCreateyAA10BufferViewVADnF : $@convention(thin) (@owned BufferView) -> @lifetime(copy 0)  @owned BufferView {
 func consumeAndCreate(_ x: consuming BufferView) -> BufferView {
-  return BufferView(independent: x.ptr, x.c)
+  let bv = BufferView(independent: x.ptr, x.c)
+  return _overrideLifetime(bv, copying: x)
 }
 
 func use(_ x: borrowing BufferView) {}
@@ -175,7 +192,7 @@ struct GenericBufferView<Element> : ~Escapable {
         count: bounds.upperBound.distance(to:bounds.lowerBound) / MemoryLayout<Element>.stride
       )
       // assuming that bounds is within self
-      return unsafeLifetime(dependent: result, dependsOn: self)
+      return _overrideLifetime(result, copying: self)
     }
   }
 }