Skip to content

Commit a7e8a37

Browse files
meg-guptameg-gupta
committed
Revert "Merge pull request swiftlang#30340 from apple/revert-29812-captureconv"
This reverts commit 04c20b8, reversing changes made to 8e39711.
1 parent c315bff commit a7e8a37

23 files changed

+293
-115
lines changed

‎include/swift/SIL/TypeLowering.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -561,6 +561,8 @@ enum class CaptureKind {
561561
StorageAddress,
562562
/// A local value captured as a constant.
563563
Constant,
564+
/// A let constant captured as a pointer to storage
565+
Immutable
564566
};
565567

566568

‎include/swift/SILOptimizer/Utils/InstOptUtils.h

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -364,12 +364,19 @@ void releasePartialApplyCapturedArg(
364364
SILParameterInfo paramInfo,
365365
InstModCallbacks callbacks = InstModCallbacks());
366366

367+
void deallocPartialApplyCapturedArg(
368+
SILBuilder &builder, SILLocation loc, SILValue arg,
369+
SILParameterInfo paramInfo);
370+
367371
/// Insert destroys of captured arguments of partial_apply [stack].
368372
void insertDestroyOfCapturedArguments(
369373
PartialApplyInst *pai, SILBuilder &builder,
370374
llvm::function_ref<bool(SILValue)> shouldInsertDestroy =
371375
[](SILValue arg) -> bool { return true; });
372376

377+
void insertDeallocOfCapturedArguments(
378+
PartialApplyInst *pai, SILBuilder &builder);
379+
373380
/// This iterator 'looks through' one level of builtin expect users exposing all
374381
/// users of the looked through builtin expect instruction i.e it presents a
375382
/// view that shows all users as if there were no builtin expect instructions

‎lib/IRGen/GenFunc.cpp

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1319,9 +1319,25 @@ Optional<StackAddress> irgen::emitFunctionPartialApplication(
13191319
SmallVector<SILType, 4> argValTypes;
13201320
SmallVector<ParameterConvention, 4> argConventions;
13211321

1322+
// Go over the params and check if any of them can cause the HeapLayout to be non-fixed
1323+
// This is needed because we should not consider parameters of kind ClassPointer and Metadata sources
1324+
// If not, we may end up with missing TypeMetadata for a type dependent generic parameter
1325+
// while generating code for destructor of HeapLayout.
1326+
bool considerParameterSources = true;
1327+
for (auto param : params) {
1328+
SILType argType = IGF.IGM.silConv.getSILType(param, origType);
1329+
auto argLoweringTy = getArgumentLoweringType(argType.getASTType(), param);
1330+
auto &ti = IGF.getTypeInfoForLowered(argLoweringTy);
1331+
1332+
if (!isa<FixedTypeInfo>(ti)) {
1333+
considerParameterSources = false;
1334+
break;
1335+
}
1336+
}
1337+
13221338
// Reserve space for polymorphic bindings.
13231339
auto bindings =
1324-
NecessaryBindings::forPartialApplyForwarder(IGF.IGM, origType, subs);
1340+
NecessaryBindings::forPartialApplyForwarder(IGF.IGM, origType, subs, considerParameterSources);
13251341

13261342
if (!bindings.empty()) {
13271343
hasSingleSwiftRefcountedContext = No;
@@ -1524,7 +1540,7 @@ Optional<StackAddress> irgen::emitFunctionPartialApplication(
15241540
HeapNonFixedOffsets offsets(IGF, layout);
15251541
if (outType->isNoEscape()) {
15261542
stackAddr = IGF.emitDynamicAlloca(
1527-
IGF.IGM.Int8Ty, layout.emitSize(IGF.IGM), Alignment(16));
1543+
IGF.IGM.Int8Ty, layout.isFixedLayout() ? layout.emitSize(IGF.IGM) : offsets.getSize() , Alignment(16));
15281544
stackAddr = stackAddr->withAddress(IGF.Builder.CreateBitCast(
15291545
stackAddr->getAddress(), IGF.IGM.OpaquePtrTy));
15301546
data = stackAddr->getAddress().getAddress();

‎lib/IRGen/GenProto.cpp

Lines changed: 21 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,7 @@ class PolymorphicConvention {
112112
}
113113

114114
public:
115-
PolymorphicConvention(IRGenModule &IGM, CanSILFunctionType fnType);
115+
PolymorphicConvention(IRGenModule &IGM, CanSILFunctionType fnType, bool considerParameterSources);
116116

117117
ArrayRef<MetadataSource> getSources() const { return Sources; }
118118

@@ -179,8 +179,9 @@ class PolymorphicConvention {
179179
} // end anonymous namespace
180180

181181
PolymorphicConvention::PolymorphicConvention(IRGenModule &IGM,
182-
CanSILFunctionType fnType)
183-
: IGM(IGM), M(*IGM.getSwiftModule()), FnType(fnType) {
182+
CanSILFunctionType fnType,
183+
bool considerParameterSources = true)
184+
: IGM(IGM), M(*IGM.getSwiftModule()), FnType(fnType){
184185
initGenerics();
185186

186187
auto rep = fnType->getRepresentation();
@@ -212,16 +213,18 @@ PolymorphicConvention::PolymorphicConvention(IRGenModule &IGM,
212213
unsigned selfIndex = ~0U;
213214
auto params = fnType->getParameters();
214215

215-
// Consider 'self' first.
216-
if (fnType->hasSelfParam()) {
217-
selfIndex = params.size() - 1;
218-
considerParameter(params[selfIndex], selfIndex, true);
219-
}
216+
if (considerParameterSources) {
217+
// Consider 'self' first.
218+
if (fnType->hasSelfParam()) {
219+
selfIndex = params.size() - 1;
220+
considerParameter(params[selfIndex], selfIndex, true);
221+
}
220222

221-
// Now consider the rest of the parameters.
222-
for (auto index : indices(params)) {
223-
if (index != selfIndex)
224-
considerParameter(params[index], index, false);
223+
// Now consider the rest of the parameters.
224+
for (auto index : indices(params)) {
225+
if (index != selfIndex)
226+
considerParameter(params[index], index, false);
227+
}
225228
}
226229
}
227230
}
@@ -2975,14 +2978,16 @@ NecessaryBindings::forFunctionInvocations(IRGenModule &IGM,
29752978
NecessaryBindings
29762979
NecessaryBindings::forPartialApplyForwarder(IRGenModule &IGM,
29772980
CanSILFunctionType origType,
2978-
SubstitutionMap subs) {
2981+
SubstitutionMap subs,
2982+
bool considerParameterSources) {
29792983
return computeBindings(IGM, origType, subs,
2980-
true /*forPartialApplyForwarder*/);
2984+
true /*forPartialApplyForwarder*/,
2985+
considerParameterSources);
29812986
}
29822987

29832988
NecessaryBindings NecessaryBindings::computeBindings(
29842989
IRGenModule &IGM, CanSILFunctionType origType, SubstitutionMap subs,
2985-
bool forPartialApplyForwarder) {
2990+
bool forPartialApplyForwarder, bool considerParameterSources) {
29862991

29872992
NecessaryBindings bindings;
29882993
bindings.forPartialApply = forPartialApplyForwarder;
@@ -2992,7 +2997,7 @@ NecessaryBindings NecessaryBindings::computeBindings(
29922997
return bindings;
29932998

29942999
// Figure out what we're actually required to pass:
2995-
PolymorphicConvention convention(IGM, origType);
3000+
PolymorphicConvention convention(IGM, origType, considerParameterSources);
29963001

29973002
// - unfulfilled requirements
29983003
convention.enumerateUnfulfilledRequirements(

‎lib/IRGen/NecessaryBindings.h

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,8 @@ class NecessaryBindings {
5959
SubstitutionMap subs);
6060
static NecessaryBindings forPartialApplyForwarder(IRGenModule &IGM,
6161
CanSILFunctionType origType,
62-
SubstitutionMap subs);
62+
SubstitutionMap subs,
63+
bool considerParameterSources = true);
6364

6465
/// Add whatever information is necessary to reconstruct type metadata
6566
/// for the given type.
@@ -98,7 +99,8 @@ class NecessaryBindings {
9899
static NecessaryBindings computeBindings(IRGenModule &IGM,
99100
CanSILFunctionType origType,
100101
SubstitutionMap subs,
101-
bool forPartialApplyForwarder);
102+
bool forPartialApplyForwarder,
103+
bool considerParameterSources = true);
102104
};
103105

104106
} // end namespace irgen

‎lib/SIL/SILFunctionType.cpp

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1464,9 +1464,8 @@ lowerCaptureContextParameters(TypeConverter &TC, SILDeclRef function,
14641464
case CaptureKind::Constant: {
14651465
// Constants are captured by value.
14661466
ParameterConvention convention;
1467-
if (loweredTL.isAddressOnly()) {
1468-
convention = ParameterConvention::Indirect_In_Guaranteed;
1469-
} else if (loweredTL.isTrivial()) {
1467+
assert (!loweredTL.isAddressOnly());
1468+
if (loweredTL.isTrivial()) {
14701469
convention = ParameterConvention::Direct_Unowned;
14711470
} else {
14721471
convention = ParameterConvention::Direct_Guaranteed;
@@ -1499,6 +1498,16 @@ lowerCaptureContextParameters(TypeConverter &TC, SILDeclRef function,
14991498
inputs.push_back(param);
15001499
break;
15011500
}
1501+
case CaptureKind::Immutable: {
1502+
// 'let' constants that are address-only are captured as the address of the value
1503+
// and will be consumed by the closure.
1504+
SILType ty = loweredTy.getAddressType();
1505+
auto param =
1506+
SILParameterInfo(ty.getASTType(),
1507+
ParameterConvention::Indirect_In_Guaranteed);
1508+
inputs.push_back(param);
1509+
break;
1510+
}
15021511
}
15031512
}
15041513
}

‎lib/SIL/SILVerifier.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1117,7 +1117,7 @@ class SILVerifier : public SILVerifierBase<SILVerifier> {
11171117
if (VarInfo)
11181118
if (unsigned ArgNo = VarInfo->ArgNo) {
11191119
// It is a function argument.
1120-
if (ArgNo < DebugVars.size() && !DebugVars[ArgNo].empty()) {
1120+
if (ArgNo < DebugVars.size() && !DebugVars[ArgNo].empty() && !VarInfo->Name.empty()) {
11211121
require(
11221122
DebugVars[ArgNo] == VarInfo->Name,
11231123
"Scope contains conflicting debug variables for one function "

‎lib/SIL/TypeLowering.cpp

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -126,6 +126,11 @@ CaptureKind TypeConverter::getDeclCaptureKind(CapturedValue capture,
126126
return CaptureKind::Box;
127127
}
128128

129+
// For 'let' constants
130+
if (!var->supportsMutation()) {
131+
return CaptureKind::Immutable;
132+
}
133+
129134
// If we're capturing into a non-escaping closure, we can generally just
130135
// capture the address of the value as no-escape.
131136
return (capture.isNoEscape()

‎lib/SILGen/SILGenFunction.cpp

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -262,6 +262,7 @@ void SILGenFunction::emitCaptures(SILLocation loc,
262262
case CaptureKind::Constant:
263263
capturedArgs.push_back(emitUndef(getLoweredType(type)));
264264
break;
265+
case CaptureKind::Immutable:
265266
case CaptureKind::StorageAddress:
266267
capturedArgs.push_back(emitUndef(getLoweredType(type).getAddressType()));
267268
break;
@@ -333,7 +334,23 @@ void SILGenFunction::emitCaptures(SILLocation loc,
333334
capturedArgs.push_back(emitManagedRValueWithCleanup(Val));
334335
break;
335336
}
336-
337+
case CaptureKind::Immutable: {
338+
if (canGuarantee) {
339+
auto entryValue = getAddressValue(Entry.value);
340+
// No-escaping stored declarations are captured as the
341+
// address of the value.
342+
assert(entryValue->getType().isAddress() && "no address for captured var!");
343+
capturedArgs.push_back(ManagedValue::forLValue(entryValue));
344+
}
345+
else {
346+
auto entryValue = getAddressValue(Entry.value);
347+
auto addr = emitTemporaryAllocation(vd, entryValue->getType());
348+
auto val = B.createCopyAddr(loc, entryValue, addr, IsNotTake,
349+
IsInitialization);
350+
capturedArgs.push_back(ManagedValue::forLValue(addr));
351+
}
352+
break;
353+
}
337354
case CaptureKind::StorageAddress: {
338355
auto entryValue = getAddressValue(Entry.value);
339356
// No-escaping stored declarations are captured as the

‎lib/SILGen/SILGenProlog.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -420,6 +420,7 @@ static void emitCaptureArguments(SILGenFunction &SGF,
420420
SGF.B.createDebugValueAddr(Loc, addr, DbgVar);
421421
break;
422422
}
423+
case CaptureKind::Immutable:
423424
case CaptureKind::StorageAddress: {
424425
// Non-escaping stored decls are captured as the address of the value.
425426
auto type = getVarTypeInCaptureContext();

‎lib/SILOptimizer/Mandatory/ClosureLifetimeFixup.cpp

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -459,6 +459,27 @@ static bool tryRewriteToPartialApplyStack(
459459
saveDeleteInst(convertOrPartialApply);
460460
saveDeleteInst(origPA);
461461

462+
ApplySite site(newPA);
463+
SILFunctionConventions calleeConv(site.getSubstCalleeType(),
464+
newPA->getModule());
465+
466+
// Since we create temporary allocation for in_guaranteed captures during SILGen,
467+
// the dealloc_stack of it can occur before the apply due to conversion scopes.
468+
// When we insert destroy_addr of the in_guaranteed capture after the apply,
469+
// we may end up with a situation when the dealloc_stack occurs before the destroy_addr.
470+
// The code below proactively removes the dealloc_stack of in_guaranteed capture,
471+
// so that it can be reinserted at the correct place after the destroy_addr below.
472+
for (auto &arg : newPA->getArgumentOperands()) {
473+
unsigned calleeArgumentIndex = site.getCalleeArgIndex(arg);
474+
assert(calleeArgumentIndex >= calleeConv.getSILArgIndexOfFirstParam());
475+
auto paramInfo = calleeConv.getParamInfoForSILArg(calleeArgumentIndex);
476+
if (paramInfo.getConvention() == ParameterConvention::Indirect_In_Guaranteed)
477+
// go over all the dealloc_stack, remove it
478+
for (auto *use : arg.get()->getUses())
479+
if (auto *deallocInst = dyn_cast<DeallocStackInst>(use->getUser()))
480+
deallocInst->eraseFromParent();
481+
}
482+
462483
// Insert destroys of arguments after the apply and the dealloc_stack.
463484
if (auto *apply = dyn_cast<ApplyInst>(singleApplyUser)) {
464485
auto insertPt = std::next(SILBasicBlock::iterator(apply));
@@ -468,11 +489,15 @@ static bool tryRewriteToPartialApplyStack(
468489
SILBuilderWithScope b3(insertPt);
469490
b3.createDeallocStack(loc, newPA);
470491
insertDestroyOfCapturedArguments(newPA, b3);
492+
// dealloc_stack of the in_guaranteed capture is inserted
493+
insertDeallocOfCapturedArguments(newPA, b3);
471494
} else if (auto *tai = dyn_cast<TryApplyInst>(singleApplyUser)) {
472495
for (auto *succBB : tai->getSuccessorBlocks()) {
473496
SILBuilderWithScope b3(succBB->begin());
474497
b3.createDeallocStack(loc, newPA);
475498
insertDestroyOfCapturedArguments(newPA, b3);
499+
// dealloc_stack of the in_guaranteed capture is inserted
500+
insertDeallocOfCapturedArguments(newPA, b3);
476501
}
477502
} else {
478503
llvm_unreachable("Unknown FullApplySite instruction kind");

‎lib/SILOptimizer/Transforms/TempRValueElimination.cpp

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -147,11 +147,23 @@ bool TempRValueOptPass::collectLoads(
147147
LLVM_DEBUG(llvm::dbgs()
148148
<< " Temp use may write/destroy its source" << *user);
149149
return false;
150-
151150
case SILInstructionKind::BeginAccessInst:
152151
return cast<BeginAccessInst>(user)->getAccessKind() == SILAccessKind::Read;
153152

153+
case SILInstructionKind::MarkDependenceInst: {
154+
auto mdi = cast<MarkDependenceInst>(user);
155+
if (mdi->getBase() == address) {
156+
return true;
157+
}
158+
for (auto *mdiUseOper : mdi->getUses()) {
159+
if (!collectLoads(mdiUseOper, mdiUseOper->getUser(), mdi, srcAddr,
160+
loadInsts))
161+
return false;
162+
}
163+
return true;
164+
}
154165
case SILInstructionKind::ApplyInst:
166+
case SILInstructionKind::PartialApplyInst:
155167
case SILInstructionKind::TryApplyInst: {
156168
ApplySite apply(user);
157169

@@ -650,7 +662,14 @@ TempRValueOptPass::tryOptimizeStoreIntoTemp(StoreInst *si) {
650662
toDelete.push_back(fli);
651663
break;
652664
}
653-
665+
case SILInstructionKind::MarkDependenceInst: {
666+
SILBuilderWithScope builder(user);
667+
auto mdi = cast<MarkDependenceInst>(user);
668+
auto newInst = builder.createMarkDependence(user->getLoc(), mdi->getValue(), si->getSrc());
669+
mdi->replaceAllUsesWith(newInst);
670+
toDelete.push_back(user);
671+
break;
672+
}
654673
// ASSUMPTION: no operations that may be handled by this default clause can
655674
// destroy tempObj. This includes operations that load the value from memory
656675
// and release it.

‎lib/SILOptimizer/Utils/InstOptUtils.cpp

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1372,6 +1372,15 @@ void swift::releasePartialApplyCapturedArg(SILBuilder &builder, SILLocation loc,
13721372
callbacks.createdNewInst(u.get<ReleaseValueInst *>());
13731373
}
13741374

1375+
void swift::deallocPartialApplyCapturedArg(SILBuilder &builder, SILLocation loc,
1376+
SILValue arg,
1377+
SILParameterInfo paramInfo) {
1378+
if (!paramInfo.isIndirectInGuaranteed())
1379+
return;
1380+
1381+
builder.createDeallocStack(loc, arg);
1382+
}
1383+
13751384
static bool
13761385
deadMarkDependenceUser(SILInstruction *inst,
13771386
SmallVectorImpl<SILInstruction *> &deleteInsts) {
@@ -1937,6 +1946,22 @@ void swift::insertDestroyOfCapturedArguments(
19371946
}
19381947
}
19391948

1949+
void swift::insertDeallocOfCapturedArguments(
1950+
PartialApplyInst *pai, SILBuilder &builder) {
1951+
assert(pai->isOnStack());
1952+
1953+
ApplySite site(pai);
1954+
SILFunctionConventions calleeConv(site.getSubstCalleeType(),
1955+
pai->getModule());
1956+
auto loc = RegularLocation::getAutoGeneratedLocation();
1957+
for (auto &arg : pai->getArgumentOperands()) {
1958+
unsigned calleeArgumentIndex = site.getCalleeArgIndex(arg);
1959+
assert(calleeArgumentIndex >= calleeConv.getSILArgIndexOfFirstParam());
1960+
auto paramInfo = calleeConv.getParamInfoForSILArg(calleeArgumentIndex);
1961+
deallocPartialApplyCapturedArg(builder, loc, arg.get(), paramInfo);
1962+
}
1963+
}
1964+
19401965
AbstractFunctionDecl *swift::getBaseMethod(AbstractFunctionDecl *FD) {
19411966
while (FD->getOverriddenDecl()) {
19421967
FD = FD->getOverriddenDecl();

0 commit comments

Comments
 (0)