Merge pull request #67040 from rjmccall/variadic-tuple-result-reabstraction-thunks

rjmccall · web-flow · commit 61e858535282 · 2023-06-30T13:57:48.000-04:00
Handle variadic tuples in reabstraction thunk emission
diff --git a/SwiftCompilerSources/Sources/SIL/Argument.swift b/SwiftCompilerSources/Sources/SIL/Argument.swift
@@ -141,11 +141,17 @@ public enum ArgumentConvention {
   /// indirectly is recorded in the pack type.
   case packGuaranteed
 
+  /// This argument is a pack of indirect return value addresses.  The
+  /// addresses are stored in the pack by the caller and read out by the
+  /// callee; within the callee, they are individually treated like
+  /// indirectOut arguments.
+  case packOut
+
   public var isIndirect: Bool {
     switch self {
     case .indirectIn, .indirectInGuaranteed,
          .indirectInout, .indirectInoutAliasable, .indirectOut,
-         .packInout, .packOwned, .packGuaranteed:
+         .packOut, .packInout, .packOwned, .packGuaranteed:
       return true
     case .directOwned, .directUnowned, .directGuaranteed:
       return false
@@ -159,7 +165,19 @@ public enum ArgumentConvention {
       return true
     case .directOwned, .directUnowned, .directGuaranteed,
          .indirectInout, .indirectInoutAliasable, .indirectOut,
-         .packInout:
+         .packOut, .packInout:
+      return false
+    }
+  }
+
+  public var isIndirectOut: Bool {
+    switch self {
+    case .indirectOut, .packOut:
+      return true
+    case .indirectInGuaranteed, .directGuaranteed, .packGuaranteed,
+         .indirectIn, .directOwned, .directUnowned,
+         .indirectInout, .indirectInoutAliasable,
+         .packInout, .packOwned:
       return false
     }
   }
@@ -170,7 +188,7 @@ public enum ArgumentConvention {
       return true
     case .indirectIn, .directOwned, .directUnowned,
          .indirectInout, .indirectInoutAliasable, .indirectOut,
-         .packInout, .packOwned:
+         .packOut, .packInout, .packOwned:
       return false
     }
   }
@@ -181,6 +199,7 @@ public enum ArgumentConvention {
          .indirectOut,
          .indirectInGuaranteed,
          .indirectInout,
+         .packOut,
          .packInout,
          .packOwned,
          .packGuaranteed:
@@ -207,6 +226,7 @@ public enum ArgumentConvention {
          .directUnowned,
          .directGuaranteed,
          .directOwned,
+         .packOut,
          .packOwned,
          .packGuaranteed:
       return false
@@ -233,6 +253,7 @@ extension BridgedArgumentConvention {
       case .Direct_Owned:            return .directOwned
       case .Direct_Unowned:          return .directUnowned
       case .Direct_Guaranteed:       return .directGuaranteed
+      case .Pack_Out:                return .packOut
       case .Pack_Inout:              return .packInout
       case .Pack_Owned:              return .packOwned
       case .Pack_Guaranteed:         return .packGuaranteed
diff --git a/SwiftCompilerSources/Sources/SIL/Effects.swift b/SwiftCompilerSources/Sources/SIL/Effects.swift
@@ -143,7 +143,7 @@ extension Function {
       if convention.isIndirectIn {
         // Even a `[readnone]` function can read from an indirect argument.
         result.memory.read = true
-      } else if convention == .indirectOut {
+      } else if convention.isIndirectOut {
         // Even `[readnone]` and `[readonly]` functions write to indirect results.
         result.memory.write = true
       }
@@ -546,7 +546,7 @@ public struct SideEffects : CustomStringConvertible, NoReflectionChildren {
       case .indirectInGuaranteed, .packGuaranteed:
         result.memory.write = false
         result.ownership.destroy = false
-      case .indirectOut, .packInout:
+      case .indirectOut, .packOut, .packInout:
         result.memory.read = false
         result.ownership.copy = false
         result.ownership.destroy = false
diff --git a/SwiftCompilerSources/Sources/SIL/Function.swift b/SwiftCompilerSources/Sources/SIL/Function.swift
@@ -85,9 +85,6 @@ final public class Function : CustomStringConvertible, HasShortDescription, Hash
   public var resultType: Type { bridged.getSILResultType().type }
 
   public func getArgumentConvention(for argumentIndex: Int) -> ArgumentConvention {
-    if argumentIndex < numIndirectResultArguments {
-      return .indirectOut
-    }
     return bridged.getSILArgumentConvention(argumentIndex).convention
   }
 
diff --git a/include/swift/Basic/Generators.h b/include/swift/Basic/Generators.h
@@ -146,6 +146,10 @@ class ArrayRefGenerator {
   }
 };
 
+namespace generator_details {
+template <class T> struct is_simple_generator_ref;
+}
+
 /// An abstracting reference to an existing generator.
 ///
 /// The implementation of this type holds the reference to the existing
@@ -182,7 +186,8 @@ class SimpleGeneratorRef {
   constexpr SimpleGeneratorRef() : vtable(nullptr), pointer(nullptr) {}
 
   template <class G>
-  constexpr SimpleGeneratorRef(G &generator)
+  constexpr SimpleGeneratorRef(G &generator,
+      typename std::enable_if<!generator_details::is_simple_generator_ref<G>::value, bool>::type = false)
     : vtable(&VTableImpl<G>::vtable), pointer(&generator) {}
 
   /// Test whether this generator ref was initialized with a
@@ -212,6 +217,19 @@ class SimpleGeneratorRef {
   }
 };
 
+namespace generator_details {
+
+template <class T>
+struct is_simple_generator_ref<SimpleGeneratorRef<T>> {
+  static constexpr bool value = true;
+};
+template <class T>
+struct is_simple_generator_ref {
+  static constexpr bool value = false;
+};
+
+}
+
 } // end namespace swift
 
 #endif
diff --git a/include/swift/SIL/AbstractionPattern.h b/include/swift/SIL/AbstractionPattern.h
@@ -1354,6 +1354,10 @@ class AbstractionPattern {
   llvm::Optional<AbstractionPattern>
   getVanishingTupleElementPatternType() const;
 
+  /// Does this tuple type vanish, i.e. is it flattened to a singleton
+  /// non-expansion element under substitution?
+  bool doesTupleVanish() const;
+
   static AbstractionPattern
   projectTupleElementType(const AbstractionPattern *base, size_t index) {
     return base->getTupleElementType(index);
diff --git a/include/swift/SIL/AbstractionPatternGenerators.h b/include/swift/SIL/AbstractionPatternGenerators.h
@@ -224,6 +224,11 @@ class TupleElementGenerator {
     return origEltIndex == numOrigElts;
   }
 
+  /// Does the entire original tuple vanish?
+  bool doesOrigTupleVanish() const {
+    return origTupleVanishes;
+  }
+
   /// Advance to the next orig element.
   void advance() {
     assert(!isFinished());
diff --git a/include/swift/SIL/SILArgument.h b/include/swift/SIL/SILArgument.h
@@ -28,19 +28,6 @@ class SILPhiArgument;
 class SILUndef;
 class TermInst;
 
-// Map an argument index onto a SILArgumentConvention.
-inline SILArgumentConvention
-SILFunctionConventions::getSILArgumentConvention(unsigned index) const {
-  assert(index <= getNumSILArguments());
-  if (index < getNumIndirectSILResults()) {
-    assert(silConv.loweredAddresses);
-    return SILArgumentConvention::Indirect_Out;
-  } else {
-    auto param = funcTy->getParameters()[index - getNumIndirectSILResults()];
-    return SILArgumentConvention(param.getConvention());
-  }
-}
-
 struct SILArgumentKind {
   enum innerty : std::underlying_type<ValueKind>::type {
 #define ARGUMENT(ID, PARENT) ID = unsigned(SILNodeKind::ID),
diff --git a/include/swift/SIL/SILArgumentConvention.h b/include/swift/SIL/SILArgumentConvention.h
@@ -164,6 +164,28 @@ struct SILArgumentConvention {
     }
     llvm_unreachable("covered switch isn't covered?!");
   }
+
+  /// Returns true if \p Value is an indirect-out parameter.
+  bool isIndirectOutParameter() {
+    switch (Value) {
+    case SILArgumentConvention::Indirect_Out:
+    case SILArgumentConvention::Pack_Out:
+      return true;
+
+    case SILArgumentConvention::Indirect_In:
+    case SILArgumentConvention::Indirect_In_Guaranteed:
+    case SILArgumentConvention::Indirect_Inout:
+    case SILArgumentConvention::Indirect_InoutAliasable:
+    case SILArgumentConvention::Direct_Unowned:
+    case SILArgumentConvention::Direct_Guaranteed:
+    case SILArgumentConvention::Direct_Owned:
+    case SILArgumentConvention::Pack_Inout:
+    case SILArgumentConvention::Pack_Owned:
+    case SILArgumentConvention::Pack_Guaranteed:
+      return false;
+    }
+    llvm_unreachable("covered switch isn't covered?!");
+  }
 };
 
 } // namespace swift
diff --git a/include/swift/SIL/SILBridging.h b/include/swift/SIL/SILBridging.h
@@ -219,7 +219,7 @@ struct BridgedFunction {
 
   BridgedArgumentConvention getSILArgumentConvention(SwiftInt idx) const {
     swift::SILFunctionConventions conv(getFunction()->getConventionsInContext());
-    return castToArgumentConvention(swift::SILArgumentConvention(conv.getParamInfoForSILArg(idx).getConvention()));
+    return castToArgumentConvention(conv.getSILArgumentConvention(idx));
   }
 
   swift::SILType getSILResultType() const {
diff --git a/include/swift/SIL/SILFunctionConventions.h b/include/swift/SIL/SILFunctionConventions.h
@@ -409,7 +409,6 @@ class SILFunctionConventions {
   /// Return the SIL argument convention of apply/entry argument at
   /// the given argument index.
   SILArgumentConvention getSILArgumentConvention(unsigned index) const;
-  // See SILArgument.h.
 
   /// Return the SIL type of the apply/entry argument at the given index.
   SILType getSILArgumentType(unsigned index,
diff --git a/lib/SIL/IR/AbstractionPattern.cpp b/lib/SIL/IR/AbstractionPattern.cpp
@@ -312,7 +312,7 @@ bool AbstractionPattern::matchesTuple(CanType substType) const {
       return false;
     LLVM_FALLTHROUGH;
   case Kind::Tuple: {
-    if (getVanishingTupleElementPatternType()) {
+    if (doesTupleVanish()) {
       // TODO: recurse into elements.
       return true;
     }
@@ -478,6 +478,11 @@ bool AbstractionPattern::doesTupleContainPackExpansionType() const {
   llvm_unreachable("bad kind");
 }
 
+bool AbstractionPattern::doesTupleVanish() const {
+  assert(isTuple());
+  return getVanishingTupleElementPatternType().has_value();
+}
+
 llvm::Optional<AbstractionPattern>
 AbstractionPattern::getVanishingTupleElementPatternType() const {
   if (!isTuple())
@@ -553,8 +558,7 @@ TupleElementGenerator::TupleElementGenerator(
   assert(origTupleType.isTuple());
   assert(origTupleType.matchesTuple(substType));
 
-  origTupleVanishes =
-    origTupleType.getVanishingTupleElementPatternType().has_value();
+  origTupleVanishes = origTupleType.doesTupleVanish();
   origTupleTypeIsOpaque = origTupleType.isOpaqueTuple();
   numOrigElts = origTupleType.getNumTupleElements();
 
diff --git a/lib/SIL/IR/SILArgument.cpp b/lib/SIL/IR/SILArgument.cpp
@@ -75,6 +75,42 @@ SILParameterInfo SILFunctionArgument::getKnownParameterInfo() const {
   return getFunction()->getConventions().getParamInfoForSILArg(getIndex());
 }
 
+SILArgumentConvention
+SILFunctionConventions::getSILArgumentConvention(unsigned index) const {
+  assert(index < getNumSILArguments());
+
+  // If the argument is a parameter, index into the parameters.
+  if (index >= getNumIndirectSILResults()) {
+    auto param = funcTy->getParameters()[index - getNumIndirectSILResults()];
+    return SILArgumentConvention(param.getConvention());
+  }
+
+  // If it's an indirect result, it could be either Pack_Out or
+  // Indirect_Out.
+
+  // Handle the common case of a function with no pack results.
+  if (funcTy->getNumPackResults() == 0) {
+    assert(silConv.loweredAddresses);
+    return SILArgumentConvention::Indirect_Out;
+  }
+
+  // Otherwise, we need to index into the indirect results to figure out
+  // whether the result is a pack or not, and unfortunately that is not a
+  // linear algorithm.
+  for (auto result : getIndirectSILResults()) {
+    if (index == 0) {
+      if (result.getConvention() == ResultConvention::Indirect) {
+        assert(silConv.loweredAddresses);
+        return SILArgumentConvention::Indirect_Out;
+      } else {
+        assert(result.getConvention() == ResultConvention::Pack);
+        return SILArgumentConvention::Pack_Out;
+      }
+    }
+    index--;
+  }
+  llvm_unreachable("mismatch with getNumIndirectSILResults()?");
+}
 
 //===----------------------------------------------------------------------===//
 //                              SILBlockArgument
diff --git a/lib/SILGen/Cleanup.cpp b/lib/SILGen/Cleanup.cpp
@@ -499,3 +499,35 @@ CleanupCloner::cloneForRemainingPackComponents(SILValue packAddr,
                                                    firstComponentIndex);
   return ManagedValue(packAddr, cleanup);
 }
+
+ManagedValue
+CleanupCloner::cloneForRemainingTupleComponents(SILValue tupleAddr,
+                                                CanPackType inducedPackType,
+                                                unsigned firstComponentIndex) const {
+  if (isLValue) {
+    return ManagedValue::forLValue(tupleAddr);
+  }
+
+  if (!hasCleanup) {
+    return ManagedValue::forUnmanaged(tupleAddr);
+  }
+
+  assert(!writebackBuffer.has_value());
+  bool isTrivial = true;
+  auto tupleTy = tupleAddr->getType().castTo<TupleType>();
+  for (auto eltTy : tupleTy.getElementTypes().slice(firstComponentIndex)) {
+    if (!SILType::getPrimitiveObjectType(eltTy).isTrivial(SGF.F)) {
+      isTrivial = false;
+      break;
+    }
+  }
+
+  if (isTrivial)
+    return ManagedValue::forUnmanaged(tupleAddr);
+
+  auto cleanup =
+    SGF.enterDestroyRemainingTupleElementsCleanup(tupleAddr,
+                                                  inducedPackType,
+                                                  firstComponentIndex);
+  return ManagedValue(tupleAddr, cleanup);
+}
diff --git a/lib/SILGen/Cleanup.h b/lib/SILGen/Cleanup.h
@@ -349,6 +349,9 @@ class CleanupCloner {
   ManagedValue cloneForRemainingPackComponents(SILValue packAddr,
                                                CanPackType formalPackType,
                                                unsigned firstComponentIndex) const;
+  ManagedValue cloneForRemainingTupleComponents(SILValue tupleAddr,
+                                                CanPackType inducedPackType,
+                                                unsigned firstComponentIndex) const;
 
   static void
   getClonersForRValue(SILGenFunction &SGF, const RValue &rvalue,
diff --git a/lib/SILGen/ResultPlan.cpp b/lib/SILGen/ResultPlan.cpp
@@ -1324,7 +1324,7 @@ ResultPlanPtr ResultPlanBuilder::buildForTuple(Initialization *init,
   // emit directly into the initialization.  If the orig tuple vanishes,
   // that counts as the initialization being splittable.
   if (init) {
-    bool vanishes = origType.getVanishingTupleElementPatternType().has_value();
+    bool vanishes = origType.doesTupleVanish();
     if (vanishes || init->canSplitIntoTupleElements()) {
       return ResultPlanPtr(
         new TupleInitializationResultPlan(*this, init, origType, substType,
diff --git a/lib/SILGen/SILGenApply.cpp b/lib/SILGen/SILGenApply.cpp
@@ -3417,8 +3417,7 @@ class ArgEmitter {
     // If the original parameter type is a vanishing tuple, we want to emit
     // this as if the argument source was wrapped in an extra level of
     // tuple literal.
-    bool origTupleVanishes =
-      origParamType.getVanishingTupleElementPatternType().has_value();
+    bool origTupleVanishes = origParamType.doesTupleVanish();
 
     auto substType = arg.getSubstRValueType();
 
diff --git a/lib/SILGen/SILGenFunction.cpp b/lib/SILGen/SILGenFunction.cpp
@@ -990,7 +990,7 @@ SILGenFunction::emitClosureValue(SILLocation loc, SILDeclRef constant,
 
   // Get the lowered AST types:
   //  - the original type
-  auto origFormalType = AbstractionPattern(constantInfo.LoweredType);
+  auto origFormalType = AbstractionPattern(subs, constantInfo.LoweredType);
 
   // - the substituted type
   auto substFormalType = expectedType;
diff --git a/lib/SILGen/SILGenFunction.h b/lib/SILGen/SILGenFunction.h
diff --git a/lib/SILGen/SILGenPack.cpp b/lib/SILGen/SILGenPack.cpp
diff --git a/lib/SILGen/SILGenPoly.cpp b/lib/SILGen/SILGenPoly.cpp
diff --git a/lib/SILGen/SILGenStmt.cpp b/lib/SILGen/SILGenStmt.cpp
diff --git a/lib/SILGen/TupleGenerators.h b/lib/SILGen/TupleGenerators.h
diff --git a/test/SILGen/variadic-generic-reabstract-tuple-result.swift b/test/SILGen/variadic-generic-reabstract-tuple-result.swift

Original file line number	Diff line number	Diff line change
`@@ -85,9 +85,6 @@ final public class Function : CustomStringConvertible, HasShortDescription, Hash`
`85`	`85`	`public var resultType: Type { bridged.getSILResultType().type }`
`86`	`86`
`87`	`87`	`public func getArgumentConvention(for argumentIndex: Int) -> ArgumentConvention {`
`88`		`- if argumentIndex < numIndirectResultArguments {`
`89`		`- return .indirectOut`
`90`		`- }`
`91`	`88`	`return bridged.getSILArgumentConvention(argumentIndex).convention`
`92`	`89`	`}`
`93`	`90`
Original file line number	Diff line number	Diff line change
`@@ -219,7 +219,7 @@ struct BridgedFunction {`
`219`	`219`
`220`	`220`	`BridgedArgumentConvention getSILArgumentConvention(SwiftInt idx) const {`
`221`	`221`	`swift::SILFunctionConventions conv(getFunction()->getConventionsInContext());`
`222`		`- return castToArgumentConvention(swift::SILArgumentConvention(conv.getParamInfoForSILArg(idx).getConvention()));`
	`222`	`+ return castToArgumentConvention(conv.getSILArgumentConvention(idx));`
`223`	`223`	`}`
`224`	`224`
`225`	`225`	`swift::SILType getSILResultType() const {`