Merge pull request swiftlang#30340 from apple/revert-29812-captureconv

Revert "Use in_guaranteed for let captures"

Author: rintaro

include/swift/SIL/TypeLowering.h

@@ -561,8 +561,6 @@ enum class CaptureKind {
   StorageAddress,
   /// A local value captured as a constant.
   Constant,
-  /// A let constant captured as a pointer to storage
-  Immutable
 };
 
 

include/swift/SILOptimizer/Utils/InstOptUtils.h

@@ -364,19 +364,12 @@ void releasePartialApplyCapturedArg(
     SILParameterInfo paramInfo,
     InstModCallbacks callbacks = InstModCallbacks());
 
-void deallocPartialApplyCapturedArg(
-    SILBuilder &builder, SILLocation loc, SILValue arg,
-    SILParameterInfo paramInfo);
-
 /// Insert destroys of captured arguments of partial_apply [stack].
 void insertDestroyOfCapturedArguments(
     PartialApplyInst *pai, SILBuilder &builder,
     llvm::function_ref<bool(SILValue)> shouldInsertDestroy =
         [](SILValue arg) -> bool { return true; });
 
-void insertDeallocOfCapturedArguments(
-    PartialApplyInst *pai, SILBuilder &builder);
-
 /// This iterator 'looks through' one level of builtin expect users exposing all
 /// users of the looked through builtin expect instruction i.e it presents a
 /// view that shows all users as if there were no builtin expect instructions

lib/IRGen/GenFunc.cpp

@@ -1319,25 +1319,9 @@ Optional<StackAddress> irgen::emitFunctionPartialApplication(
   SmallVector<SILType, 4> argValTypes;
   SmallVector<ParameterConvention, 4> argConventions;
 
- // Go over the params and check if any of them can cause the HeapLayout to be non-fixed
- // This is needed because we should not consider parameters of kind ClassPointer and Metadata sources
- // If not, we may end up with missing TypeMetadata for a type dependent generic parameter
- // while generating code for destructor of HeapLayout.
-  bool considerParameterSources = true;
-  for (auto param : params) {
-    SILType argType = IGF.IGM.silConv.getSILType(param, origType);
-    auto argLoweringTy = getArgumentLoweringType(argType.getASTType(), param);
-    auto &ti = IGF.getTypeInfoForLowered(argLoweringTy);
-
-    if (!isa<FixedTypeInfo>(ti)) {
-      considerParameterSources = false;
-      break;
-    }
-  }
-
   // Reserve space for polymorphic bindings.
   auto bindings =
-      NecessaryBindings::forPartialApplyForwarder(IGF.IGM, origType, subs, considerParameterSources);
+      NecessaryBindings::forPartialApplyForwarder(IGF.IGM, origType, subs);
 
   if (!bindings.empty()) {
     hasSingleSwiftRefcountedContext = No;
@@ -1540,7 +1524,7 @@ Optional<StackAddress> irgen::emitFunctionPartialApplication(
     HeapNonFixedOffsets offsets(IGF, layout);
     if (outType->isNoEscape()) {
       stackAddr = IGF.emitDynamicAlloca(
-          IGF.IGM.Int8Ty, layout.isFixedLayout() ? layout.emitSize(IGF.IGM) : offsets.getSize() , Alignment(16));
+          IGF.IGM.Int8Ty, layout.emitSize(IGF.IGM), Alignment(16));
       stackAddr = stackAddr->withAddress(IGF.Builder.CreateBitCast(
           stackAddr->getAddress(), IGF.IGM.OpaquePtrTy));
       data = stackAddr->getAddress().getAddress();

lib/IRGen/GenProto.cpp

@@ -112,7 +112,7 @@ class PolymorphicConvention {
   }
 
 public:
-  PolymorphicConvention(IRGenModule &IGM, CanSILFunctionType fnType, bool considerParameterSources);
+  PolymorphicConvention(IRGenModule &IGM, CanSILFunctionType fnType);
 
   ArrayRef<MetadataSource> getSources() const { return Sources; }
 
@@ -179,9 +179,8 @@ class PolymorphicConvention {
 } // end anonymous namespace
 
 PolymorphicConvention::PolymorphicConvention(IRGenModule &IGM,
-                                             CanSILFunctionType fnType,
-                                             bool considerParameterSources = true)
-  : IGM(IGM), M(*IGM.getSwiftModule()), FnType(fnType){
+                                             CanSILFunctionType fnType)
+  : IGM(IGM), M(*IGM.getSwiftModule()), FnType(fnType) {
   initGenerics();
 
   auto rep = fnType->getRepresentation();
@@ -213,18 +212,16 @@ PolymorphicConvention::PolymorphicConvention(IRGenModule &IGM,
     unsigned selfIndex = ~0U;
     auto params = fnType->getParameters();
 
-    if (considerParameterSources) {
-      // Consider 'self' first.
-      if (fnType->hasSelfParam()) {
-        selfIndex = params.size() - 1;
-        considerParameter(params[selfIndex], selfIndex, true);
-      }
+    // Consider 'self' first.
+    if (fnType->hasSelfParam()) {
+      selfIndex = params.size() - 1;
+      considerParameter(params[selfIndex], selfIndex, true);
+    }
 
-      // Now consider the rest of the parameters.
-      for (auto index : indices(params)) {
-        if (index != selfIndex)
-          considerParameter(params[index], index, false);
-      }
+    // Now consider the rest of the parameters.
+    for (auto index : indices(params)) {
+      if (index != selfIndex)
+        considerParameter(params[index], index, false);
     }
   }
 }
@@ -3003,16 +3000,14 @@ NecessaryBindings::forFunctionInvocations(IRGenModule &IGM,
 NecessaryBindings
 NecessaryBindings::forPartialApplyForwarder(IRGenModule &IGM,
                                           CanSILFunctionType origType,
-                                          SubstitutionMap subs,
-                                          bool considerParameterSources) {
+                                          SubstitutionMap subs) {
   return computeBindings(IGM, origType, subs,
-                         true  /*forPartialApplyForwarder*/,
-                         considerParameterSources);
+                         true /*forPartialApplyForwarder*/);
 }
 
 NecessaryBindings NecessaryBindings::computeBindings(
     IRGenModule &IGM, CanSILFunctionType origType, SubstitutionMap subs,
-    bool forPartialApplyForwarder, bool considerParameterSources) {
+    bool forPartialApplyForwarder) {
 
   NecessaryBindings bindings;
   bindings.forPartialApply = forPartialApplyForwarder;
@@ -3022,7 +3017,7 @@ NecessaryBindings NecessaryBindings::computeBindings(
     return bindings;
 
   // Figure out what we're actually required to pass:
-  PolymorphicConvention convention(IGM, origType, considerParameterSources);
+  PolymorphicConvention convention(IGM, origType);
 
   //  - unfulfilled requirements
   convention.enumerateUnfulfilledRequirements(

lib/IRGen/NecessaryBindings.h

@@ -59,8 +59,7 @@ class NecessaryBindings {
                                                   SubstitutionMap subs);
   static NecessaryBindings forPartialApplyForwarder(IRGenModule &IGM,
                                                     CanSILFunctionType origType,
-                                                    SubstitutionMap subs,
-                                                    bool considerParameterSources = true);
+                                                    SubstitutionMap subs);
 
   /// Add whatever information is necessary to reconstruct type metadata
   /// for the given type.
@@ -99,8 +98,7 @@ class NecessaryBindings {
   static NecessaryBindings computeBindings(IRGenModule &IGM,
                                            CanSILFunctionType origType,
                                            SubstitutionMap subs,
-                                           bool forPartialApplyForwarder,
-                                           bool considerParameterSources = true);
+                                           bool forPartialApplyForwarder);
 };
 
 } // end namespace irgen

lib/SIL/SILFunctionType.cpp

@@ -1460,8 +1460,9 @@ lowerCaptureContextParameters(TypeConverter &TC, SILDeclRef function,
     case CaptureKind::Constant: {
       // Constants are captured by value.
       ParameterConvention convention;
-      assert (!loweredTL.isAddressOnly());
-      if (loweredTL.isTrivial()) {
+      if (loweredTL.isAddressOnly()) {
+        convention = ParameterConvention::Indirect_In_Guaranteed;
+      } else if (loweredTL.isTrivial()) {
         convention = ParameterConvention::Direct_Unowned;
       } else {
         convention = ParameterConvention::Direct_Guaranteed;
@@ -1494,16 +1495,6 @@ lowerCaptureContextParameters(TypeConverter &TC, SILDeclRef function,
       inputs.push_back(param);
       break;
     }
-    case CaptureKind::Immutable: {
-      // 'let' constants that are address-only are captured as the address of the value
-      //  and will be consumed by the closure.
-      SILType ty = loweredTy.getAddressType();
-      auto param =
-          SILParameterInfo(ty.getASTType(),
-                           ParameterConvention::Indirect_In_Guaranteed);
-      inputs.push_back(param);
-      break;
-    }
     }
   }
 }

lib/SIL/SILVerifier.cpp

@@ -1117,7 +1117,7 @@ class SILVerifier : public SILVerifierBase<SILVerifier> {
       if (VarInfo)
         if (unsigned ArgNo = VarInfo->ArgNo) {
           // It is a function argument.
-          if (ArgNo < DebugVars.size() && !DebugVars[ArgNo].empty() && !VarInfo->Name.empty()) {
+          if (ArgNo < DebugVars.size() && !DebugVars[ArgNo].empty()) {
             require(
                 DebugVars[ArgNo] == VarInfo->Name,
                 "Scope contains conflicting debug variables for one function "

lib/SIL/TypeLowering.cpp

@@ -126,11 +126,6 @@ CaptureKind TypeConverter::getDeclCaptureKind(CapturedValue capture,
     return CaptureKind::Box;
   }
 
-  // For 'let' constants
-  if (!var->supportsMutation()) {
-    return CaptureKind::Immutable;
-  }
-
   // If we're capturing into a non-escaping closure, we can generally just
   // capture the address of the value as no-escape.
   return (capture.isNoEscape()

lib/SILGen/SILGenFunction.cpp

@@ -262,7 +262,6 @@ void SILGenFunction::emitCaptures(SILLocation loc,
       case CaptureKind::Constant:
         capturedArgs.push_back(emitUndef(getLoweredType(type)));
         break;
-      case CaptureKind::Immutable:
       case CaptureKind::StorageAddress:
         capturedArgs.push_back(emitUndef(getLoweredType(type).getAddressType()));
         break;
@@ -334,23 +333,7 @@ void SILGenFunction::emitCaptures(SILLocation loc,
       capturedArgs.push_back(emitManagedRValueWithCleanup(Val));
       break;
     }
-    case CaptureKind::Immutable: {
-      if (canGuarantee) {
-        auto entryValue = getAddressValue(Entry.value);
-        // No-escaping stored declarations are captured as the
-        // address of the value.
-        assert(entryValue->getType().isAddress() && "no address for captured var!");
-        capturedArgs.push_back(ManagedValue::forLValue(entryValue));
-      }
-      else {
-        auto entryValue = getAddressValue(Entry.value);
-        auto addr = emitTemporaryAllocation(vd, entryValue->getType());
-        auto val = B.createCopyAddr(loc, entryValue, addr, IsNotTake,
-                         IsInitialization);
-        capturedArgs.push_back(ManagedValue::forLValue(addr));
-      }
-      break;
-    }
+
     case CaptureKind::StorageAddress: {
       auto entryValue = getAddressValue(Entry.value);
       // No-escaping stored declarations are captured as the

lib/SILGen/SILGenProlog.cpp

@@ -420,7 +420,6 @@ static void emitCaptureArguments(SILGenFunction &SGF,
     SGF.B.createDebugValueAddr(Loc, addr, DbgVar);
     break;
   }
-  case CaptureKind::Immutable:
   case CaptureKind::StorageAddress: {
     // Non-escaping stored decls are captured as the address of the value.
     auto type = getVarTypeInCaptureContext();