Merge pull request #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();

lib/SILOptimizer/Mandatory/ClosureLifetimeFixup.cpp

@@ -459,27 +459,6 @@ static bool tryRewriteToPartialApplyStack(
     saveDeleteInst(convertOrPartialApply);
   saveDeleteInst(origPA);
 
-  ApplySite site(newPA);
-  SILFunctionConventions calleeConv(site.getSubstCalleeType(),
-                                      newPA->getModule());
-
-  // Since we create temporary allocation for in_guaranteed captures during SILGen,
-  // the dealloc_stack of it can occur before the apply due to conversion scopes.
-  // When we insert destroy_addr of the in_guaranteed capture after the apply,
-  // we may end up with a situation when the dealloc_stack occurs before the destroy_addr.
-  // The code below proactively removes the dealloc_stack of in_guaranteed capture,
-  // so that it can be reinserted at the correct place after the destroy_addr below.
-  for (auto &arg : newPA->getArgumentOperands()) {
-    unsigned calleeArgumentIndex = site.getCalleeArgIndex(arg);
-    assert(calleeArgumentIndex >= calleeConv.getSILArgIndexOfFirstParam());
-    auto paramInfo = calleeConv.getParamInfoForSILArg(calleeArgumentIndex);
-    if (paramInfo.getConvention() == ParameterConvention::Indirect_In_Guaranteed)
-      // go over all the dealloc_stack, remove it
-      for (auto *use : arg.get()->getUses())
-        if (auto *deallocInst = dyn_cast<DeallocStackInst>(use->getUser()))
-          deallocInst->eraseFromParent();
-  }
-
   // Insert destroys of arguments after the apply and the dealloc_stack.
   if (auto *apply = dyn_cast<ApplyInst>(singleApplyUser)) {
     auto insertPt = std::next(SILBasicBlock::iterator(apply));
@@ -489,15 +468,11 @@ static bool tryRewriteToPartialApplyStack(
     SILBuilderWithScope b3(insertPt);
     b3.createDeallocStack(loc, newPA);
     insertDestroyOfCapturedArguments(newPA, b3);
-    // dealloc_stack of the in_guaranteed capture is inserted
-    insertDeallocOfCapturedArguments(newPA, b3);
   } else if (auto *tai = dyn_cast<TryApplyInst>(singleApplyUser)) {
     for (auto *succBB : tai->getSuccessorBlocks()) {
       SILBuilderWithScope b3(succBB->begin());
       b3.createDeallocStack(loc, newPA);
       insertDestroyOfCapturedArguments(newPA, b3);
-      // dealloc_stack of the in_guaranteed capture is inserted
-      insertDeallocOfCapturedArguments(newPA, b3);
     }
   } else {
     llvm_unreachable("Unknown FullApplySite instruction kind");

lib/SILOptimizer/Transforms/TempRValueElimination.cpp

@@ -147,23 +147,11 @@ bool TempRValueOptPass::collectLoads(
     LLVM_DEBUG(llvm::dbgs()
                << "  Temp use may write/destroy its source" << *user);
     return false;
+
   case SILInstructionKind::BeginAccessInst:
     return cast<BeginAccessInst>(user)->getAccessKind() == SILAccessKind::Read;
 
-  case SILInstructionKind::MarkDependenceInst: {
-    auto mdi = cast<MarkDependenceInst>(user);
-    if (mdi->getBase() == address) {
-      return true;
-    }
-    for (auto *mdiUseOper : mdi->getUses()) {
-      if (!collectLoads(mdiUseOper, mdiUseOper->getUser(), mdi, srcAddr,
-                        loadInsts))
-        return false;
-    }
-    return true;
-  }
   case SILInstructionKind::ApplyInst:
-  case SILInstructionKind::PartialApplyInst:
   case SILInstructionKind::TryApplyInst: {
     ApplySite apply(user);
 
@@ -662,14 +650,7 @@ TempRValueOptPass::tryOptimizeStoreIntoTemp(StoreInst *si) {
       toDelete.push_back(fli);
       break;
     }
-    case SILInstructionKind::MarkDependenceInst: {
-      SILBuilderWithScope builder(user);
-      auto mdi = cast<MarkDependenceInst>(user);
-      auto newInst = builder.createMarkDependence(user->getLoc(), mdi->getValue(), si->getSrc());
-      mdi->replaceAllUsesWith(newInst);
-      toDelete.push_back(user);
-      break;
-    }
+
     // ASSUMPTION: no operations that may be handled by this default clause can
     // destroy tempObj. This includes operations that load the value from memory
     // and release it.

lib/SILOptimizer/Utils/InstOptUtils.cpp

@@ -1372,15 +1372,6 @@ void swift::releasePartialApplyCapturedArg(SILBuilder &builder, SILLocation loc,
   callbacks.createdNewInst(u.get<ReleaseValueInst *>());
 }
 
-void swift::deallocPartialApplyCapturedArg(SILBuilder &builder, SILLocation loc,
-                                           SILValue arg,
-                                           SILParameterInfo paramInfo) {
-  if (!paramInfo.isIndirectInGuaranteed())
-    return;
-
-  builder.createDeallocStack(loc, arg);
-}
-
 static bool
 deadMarkDependenceUser(SILInstruction *inst,
                        SmallVectorImpl<SILInstruction *> &deleteInsts) {
@@ -1946,22 +1937,6 @@ void swift::insertDestroyOfCapturedArguments(
   }
 }
 
-void swift::insertDeallocOfCapturedArguments(
-    PartialApplyInst *pai, SILBuilder &builder) {
-  assert(pai->isOnStack());
-
-  ApplySite site(pai);
-  SILFunctionConventions calleeConv(site.getSubstCalleeType(),
-                                    pai->getModule());
-  auto loc = RegularLocation::getAutoGeneratedLocation();
-  for (auto &arg : pai->getArgumentOperands()) {
-    unsigned calleeArgumentIndex = site.getCalleeArgIndex(arg);
-    assert(calleeArgumentIndex >= calleeConv.getSILArgIndexOfFirstParam());
-    auto paramInfo = calleeConv.getParamInfoForSILArg(calleeArgumentIndex);
-    deallocPartialApplyCapturedArg(builder, loc, arg.get(), paramInfo);
-  }
-}
-
 AbstractFunctionDecl *swift::getBaseMethod(AbstractFunctionDecl *FD) {
   while (FD->getOverriddenDecl()) {
     FD = FD->getOverriddenDecl();