Skip to content

[sil-combine] Use high level ARC APIs instead of low level ARC APIs in a few cases to make code work in ossa and non-ossa. #35046

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 15 additions & 13 deletions lib/SILOptimizer/SILCombiner/SILCombinerApplyVisitors.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -945,8 +945,8 @@ struct ConcreteArgumentCopy {
}

static Optional<ConcreteArgumentCopy>
generate(const ConcreteExistentialInfo &CEI, ApplySite apply, unsigned argIdx,
SILBuilderContext &BuilderCtx) {
generate(const ConcreteExistentialInfo &existentialInfo, ApplySite apply,
unsigned argIdx, SILBuilderContext &builderCtx) {
SILParameterInfo paramInfo =
apply.getOrigCalleeConv().getParamInfoForSILArg(argIdx);
// Mutation should have been checked before we get this far.
Expand Down Expand Up @@ -976,21 +976,23 @@ struct ConcreteArgumentCopy {
if (!paramInfo.isFormalIndirect())
return None;

SILBuilderWithScope B(apply.getInstruction(), BuilderCtx);
SILBuilderWithScope builder(apply.getInstruction(), builderCtx);
auto loc = apply.getLoc();
auto *ASI = B.createAllocStack(loc, CEI.ConcreteValue->getType());
auto *asi =
builder.createAllocStack(loc, existentialInfo.ConcreteValue->getType());
// If the type is an address, simple copy it.
if (CEI.ConcreteValue->getType().isAddress()) {
B.createCopyAddr(loc, CEI.ConcreteValue, ASI, IsNotTake,
IsInitialization_t::IsInitialization);
if (existentialInfo.ConcreteValue->getType().isAddress()) {
builder.createCopyAddr(loc, existentialInfo.ConcreteValue, asi, IsNotTake,
IsInitialization_t::IsInitialization);
} else {
// Otherwise, we probably got the value from the source of a store
// instruction so, create a store into the temporary argument.
B.createStrongRetain(loc, CEI.ConcreteValue, B.getDefaultAtomicity());
B.createStore(loc, CEI.ConcreteValue, ASI,
StoreOwnershipQualifier::Unqualified);
auto copy =
builder.emitCopyValueOperation(loc, existentialInfo.ConcreteValue);
builder.emitStoreValueOperation(loc, copy, asi,
StoreOwnershipQualifier::Init);
}
return ConcreteArgumentCopy(origArg, ASI);
return ConcreteArgumentCopy(origArg, asi);
}
};

Expand Down Expand Up @@ -1416,7 +1418,7 @@ static void emitMatchingRCAdjustmentsForCall(ApplyInst *Call, SILValue OnX) {

// Emit a retain for the @owned return.
SILBuilderWithScope Builder(Call);
Builder.createRetainValue(Call->getLoc(), OnX, Builder.getDefaultAtomicity());
OnX = Builder.emitCopyValueOperation(Call->getLoc(), OnX);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One thing to be aware of is that when we compile without ownership, we return the retain_value's operand here.


// Emit a release for the @owned parameter, or none for a @guaranteed
// parameter.
Expand All @@ -1428,7 +1430,7 @@ static void emitMatchingRCAdjustmentsForCall(ApplyInst *Call, SILValue OnX) {
ParamInfo == ParameterConvention::Direct_Guaranteed);

if (ParamInfo == ParameterConvention::Direct_Owned)
Builder.createReleaseValue(Call->getLoc(), OnX, Builder.getDefaultAtomicity());
Builder.emitDestroyValueOperation(Call->getLoc(), OnX);
}

/// Replace an application of a cast composition f_inverse(f(x)) by x.
Expand Down
3 changes: 1 addition & 2 deletions lib/SILOptimizer/SILCombiner/SILCombinerMiscVisitors.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -85,8 +85,7 @@ SILCombiner::visitAllocExistentialBoxInst(AllocExistentialBoxInst *AEBI) {
// retain_value %value // must insert the release after this retain
// strong_release %box
Builder.setInsertionPoint(singleRelease);
Builder.createReleaseValue(AEBI->getLoc(), boxedValue,
singleRelease->getAtomicity());
Builder.emitDestroyValueOperation(AEBI->getLoc(), boxedValue);

eraseInstIncludingUsers(AEBI);
return nullptr;
Expand Down