Skip to content

[semantic-arc] Handle the rest of the unqualified mem opts in SILGen. #5692

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
12 changes: 12 additions & 0 deletions include/swift/SIL/SILBuilder.h
Original file line number Diff line number Diff line change
Expand Up @@ -459,6 +459,12 @@ class SILBuilder {

LoadInst *createLoad(SILLocation Loc, SILValue LV,
LoadOwnershipQualifier Qualifier) {
assert((Qualifier != LoadOwnershipQualifier::Unqualified) ||
F.hasUnqualifiedOwnership() &&
"Unqualified inst in qualified function");
assert((Qualifier == LoadOwnershipQualifier::Unqualified) ||
F.hasQualifiedOwnership() &&
"Qualified inst in unqualified function");
assert(LV->getType().isLoadable(F.getModule()));
return insert(new (F.getModule())
LoadInst(getSILDebugLocation(Loc), LV, Qualifier));
Expand All @@ -481,6 +487,12 @@ class SILBuilder {

StoreInst *createStore(SILLocation Loc, SILValue Src, SILValue DestAddr,
StoreOwnershipQualifier Qualifier) {
assert((Qualifier != StoreOwnershipQualifier::Unqualified) ||
F.hasUnqualifiedOwnership() &&
"Unqualified inst in qualified function");
assert((Qualifier == StoreOwnershipQualifier::Unqualified) ||
F.hasQualifiedOwnership() &&
"Qualified inst in unqualified function");
return insert(new (F.getModule()) StoreInst(getSILDebugLocation(Loc), Src,
DestAddr, Qualifier));
}
Expand Down
4 changes: 3 additions & 1 deletion include/swift/SIL/SILValue.h
Original file line number Diff line number Diff line change
Expand Up @@ -301,9 +301,11 @@ class OperandValueArrayRef {
: Operands(operands) {}

/// A simple iterator adapter.
class iterator {
class iterator : public std::iterator<std::forward_iterator_tag,
SILValue, ptrdiff_t> {
const Operand *Ptr;
public:
iterator() = default;
iterator(const Operand *ptr) : Ptr(ptr) {}
SILValue operator*() const { assert(Ptr); return Ptr->get(); }
SILValue operator->() const { return operator*(); }
Expand Down
8 changes: 5 additions & 3 deletions lib/Parse/ParseSIL.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3996,16 +3996,18 @@ bool SILParser::parseSILBasicBlock(SILBuilder &B) {

bool AssumeUnqualifiedOwnershipWhenParsing =
F->getModule().getOptions().AssumeUnqualifiedOwnershipWhenParsing;
if (AssumeUnqualifiedOwnershipWhenParsing) {
F->setUnqualifiedOwnership();
}
do {
if (parseSILInstruction(BB, B))
return true;
// Evaluate how the just parsed instruction effects this functions Ownership
// Qualification. For more details, see the comment on the
// FunctionOwnershipEvaluator class.
SILInstruction *ParsedInst = &*BB->rbegin();
if (AssumeUnqualifiedOwnershipWhenParsing) {
F->setUnqualifiedOwnership();
} else if (!OwnershipEvaluator.evaluate(ParsedInst)) {
if (!AssumeUnqualifiedOwnershipWhenParsing &&
!OwnershipEvaluator.evaluate(ParsedInst)) {
P.diagnose(ParsedInst->getLoc().getSourceLoc(),
diag::found_unqualified_instruction_in_qualified_function,
F->getName());
Expand Down
35 changes: 16 additions & 19 deletions lib/SIL/SILVerifier.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1123,33 +1123,30 @@ class SILVerifier : public SILVerifierBase<SILVerifier> {
case LoadOwnershipQualifier::Unqualified:
// We should not see loads with unqualified ownership when SILOwnership is
// enabled.
requireTrueAndSILOwnership(
this, F.hasUnqualifiedOwnership(),
"Load with unqualified ownership in a qualified function");
require(F.hasUnqualifiedOwnership(),
"Load with unqualified ownership in a qualified function");
break;
case LoadOwnershipQualifier::Copy:
case LoadOwnershipQualifier::Take:
requireTrueAndSILOwnership(
this, F.hasQualifiedOwnership(),
"Load with qualified ownership in an unqualified function");
require(F.hasQualifiedOwnership(),
"Load with qualified ownership in an unqualified function");
// TODO: Could probably make this a bit stricter.
require(!LI->getType().isTrivial(LI->getModule()),
"load [copy] or load [take] can only be applied to non-trivial "
"types");
break;
case LoadOwnershipQualifier::Trivial:
requireTrueAndSILOwnership(
this, F.hasQualifiedOwnership(),
"Load with qualified ownership in an unqualified function");
require(F.hasQualifiedOwnership(),
"Load with qualified ownership in an unqualified function");
require(LI->getType().isTrivial(LI->getModule()),
"A load with trivial ownership must load a trivial type");
break;
}
}

void checkLoadBorrowInst(LoadBorrowInst *LBI) {
requireTrueAndSILOwnership(
this, F.hasQualifiedOwnership(),
require(
F.hasQualifiedOwnership(),
"Inst with qualified ownership in a function that is not qualified");
require(LBI->getType().isObject(), "Result of load must be an object");
require(LBI->getType().isLoadable(LBI->getModule()),
Expand All @@ -1161,8 +1158,8 @@ class SILVerifier : public SILVerifierBase<SILVerifier> {
}

void checkEndBorrowInst(EndBorrowInst *EBI) {
requireTrueAndSILOwnership(
this, F.hasQualifiedOwnership(),
require(
F.hasQualifiedOwnership(),
"Inst with qualified ownership in a function that is not qualified");
// We allow for end_borrow to express relationships in between addresses and
// values, but we require that the types are the same ignoring value
Expand All @@ -1188,22 +1185,22 @@ class SILVerifier : public SILVerifierBase<SILVerifier> {
case StoreOwnershipQualifier::Unqualified:
// We should not see loads with unqualified ownership when SILOwnership is
// enabled.
requireTrueAndSILOwnership(this, F.hasUnqualifiedOwnership(),
"Invalid load with unqualified ownership");
require(F.hasUnqualifiedOwnership(),
"Qualified store in function with unqualified ownership?!");
break;
case StoreOwnershipQualifier::Init:
case StoreOwnershipQualifier::Assign:
requireTrueAndSILOwnership(
this, F.hasQualifiedOwnership(),
require(
F.hasQualifiedOwnership(),
"Inst with qualified ownership in a function that is not qualified");
// TODO: Could probably make this a bit stricter.
require(!SI->getSrc()->getType().isTrivial(SI->getModule()),
"store [init] or store [assign] can only be applied to "
"non-trivial types");
break;
case StoreOwnershipQualifier::Trivial:
requireTrueAndSILOwnership(
this, F.hasQualifiedOwnership(),
require(
F.hasQualifiedOwnership(),
"Inst with qualified ownership in a function that is not qualified");
require(SI->getSrc()->getType().isTrivial(SI->getModule()),
"A store with trivial ownership must store a trivial type");
Expand Down
68 changes: 49 additions & 19 deletions lib/SIL/TypeLowering.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -494,8 +494,7 @@ namespace {
public:
void emitDestroyAddress(SILBuilder &B, SILLocation loc,
SILValue addr) const override {
SILValue value =
B.createLoad(loc, addr, LoadOwnershipQualifier::Unqualified);
SILValue value = emitLoad(B, loc, addr, LoadOwnershipQualifier::Take);
emitDestroyValue(B, loc, value);
}

Expand All @@ -520,23 +519,29 @@ namespace {

SILValue emitLoadOfCopy(SILBuilder &B, SILLocation loc, SILValue addr,
IsTake_t isTake) const override {
return B.createLoad(loc, addr, LoadOwnershipQualifier::Unqualified);
return emitLoad(B, loc, addr, LoadOwnershipQualifier::Trivial);
}

void emitStoreOfCopy(SILBuilder &B, SILLocation loc,
SILValue value, SILValue addr,
IsInitialization_t isInit) const override {
B.createStore(loc, value, addr, StoreOwnershipQualifier::Unqualified);
emitStore(B, loc, value, addr, StoreOwnershipQualifier::Trivial);
}

void emitStore(SILBuilder &B, SILLocation loc, SILValue value,
SILValue addr, StoreOwnershipQualifier qual) const override {
B.createStore(loc, value, addr, StoreOwnershipQualifier::Trivial);
if (B.getFunction().hasQualifiedOwnership()) {
B.createStore(loc, value, addr, StoreOwnershipQualifier::Trivial);
return;
}
B.createStore(loc, value, addr, StoreOwnershipQualifier::Unqualified);
}

SILValue emitLoad(SILBuilder &B, SILLocation loc, SILValue addr,
LoadOwnershipQualifier qual) const override {
return B.createLoad(loc, addr, LoadOwnershipQualifier::Trivial);
if (B.getFunction().hasQualifiedOwnership())
return B.createLoad(loc, addr, LoadOwnershipQualifier::Trivial);
return B.createLoad(loc, addr, LoadOwnershipQualifier::Unqualified);
}

void emitDestroyAddress(SILBuilder &B, SILLocation loc,
Expand Down Expand Up @@ -576,32 +581,57 @@ namespace {

SILValue emitLoadOfCopy(SILBuilder &B, SILLocation loc,
SILValue addr, IsTake_t isTake) const override {
SILValue value =
B.createLoad(loc, addr, LoadOwnershipQualifier::Unqualified);
if (!isTake)
emitCopyValue(B, loc, value);
return value;
auto qual =
isTake ? LoadOwnershipQualifier::Take : LoadOwnershipQualifier::Copy;
return emitLoad(B, loc, addr, qual);
}

void emitStoreOfCopy(SILBuilder &B, SILLocation loc,
SILValue newValue, SILValue addr,
IsInitialization_t isInit) const override {
SILValue oldValue;
if (!isInit)
oldValue = B.createLoad(loc, addr, LoadOwnershipQualifier::Unqualified);
B.createStore(loc, newValue, addr, StoreOwnershipQualifier::Unqualified);
if (!isInit)
emitDestroyValue(B, loc, oldValue);
auto qual = isInit ? StoreOwnershipQualifier::Init
: StoreOwnershipQualifier::Assign;
emitStore(B, loc, newValue, addr, qual);
}

void emitStore(SILBuilder &B, SILLocation loc, SILValue value,
SILValue addr, StoreOwnershipQualifier qual) const override {
B.createStore(loc, value, addr, qual);
if (B.getFunction().hasQualifiedOwnership()) {
B.createStore(loc, value, addr, qual);
return;
}

if (qual != StoreOwnershipQualifier::Assign) {
B.createStore(loc, value, addr, StoreOwnershipQualifier::Unqualified);
return;
}

// If the ownership qualifier is [assign], then we need to eliminate the
// old value.
//
// 1. Load old value.
// 2. Store new value.
// 3. Release old value.
SILValue old =
B.createLoad(loc, addr, LoadOwnershipQualifier::Unqualified);
B.createStore(loc, value, addr, StoreOwnershipQualifier::Unqualified);
B.emitDestroyValueOperation(loc, old);
}

SILValue emitLoad(SILBuilder &B, SILLocation loc, SILValue addr,
LoadOwnershipQualifier qual) const override {
return B.createLoad(loc, addr, qual);
if (B.getFunction().hasQualifiedOwnership())
return B.createLoad(loc, addr, qual);

SILValue loadValue =
B.createLoad(loc, addr, LoadOwnershipQualifier::Unqualified);

// If we do not have a copy, just return the value...
if (qual != LoadOwnershipQualifier::Copy)
return loadValue;

// Otherwise, emit the copy value operation.
return B.emitCopyValueOperation(loc, loadValue);
}
};

Expand Down
2 changes: 1 addition & 1 deletion lib/SILGen/RValue.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ class ExplodeTupleValue
// loaded. Except it's not really an invariant, because
// argument emission likes to lie sometimes.
if (eltTI.isLoadable()) {
elt = gen.B.createLoad(loc, elt, LoadOwnershipQualifier::Unqualified);
elt = eltTI.emitLoad(gen.B, loc, elt, LoadOwnershipQualifier::Take);
}
}

Expand Down
16 changes: 8 additions & 8 deletions lib/SILGen/SILGenApply.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1999,8 +1999,8 @@ namespace {

// If the value isn't address-only, go ahead and load.
if (!substTL.isAddressOnly()) {
auto load = gen.B.createLoad(loc, value.forward(gen),
LoadOwnershipQualifier::Unqualified);
auto load = substTL.emitLoad(gen.B, loc, value.forward(gen),
LoadOwnershipQualifier::Take);
value = gen.emitManagedRValueWithCleanup(load);
}

Expand Down Expand Up @@ -3908,14 +3908,14 @@ ManagedValue SILGenFunction::emitInjectEnum(SILLocation loc,
if (payloadMV) {
// If the payload was indirect, we already evaluated it and
// have a single value. Store it into the result.
B.createStore(loc, payloadMV.forward(*this), resultData,
StoreOwnershipQualifier::Unqualified);
B.emitStoreValueOperation(loc, payloadMV.forward(*this), resultData,
StoreOwnershipQualifier::Init);
} else if (payloadTL.isLoadable()) {
// The payload of this specific enum case might be loadable
// even if the overall enum is address-only.
payloadMV = std::move(payload).getAsSingleValue(*this, origFormalType);
B.createStore(loc, payloadMV.forward(*this), resultData,
StoreOwnershipQualifier::Unqualified);
B.emitStoreValueOperation(loc, payloadMV.forward(*this), resultData,
StoreOwnershipQualifier::Init);
} else {
// The payload is address-only. Evaluate it directly into
// the enum.
Expand Down Expand Up @@ -5573,7 +5573,7 @@ RValue SILGenFunction::emitDynamicMemberRefExpr(DynamicMemberRefExpr *e,
// Package up the result.
auto optResult = optTemp;
if (optTL.isLoadable())
optResult = B.createLoad(e, optResult, LoadOwnershipQualifier::Unqualified);
optResult = optTL.emitLoad(B, e, optResult, LoadOwnershipQualifier::Take);
return RValue(*this, e, emitManagedRValueWithCleanup(optResult, optTL));
}

Expand Down Expand Up @@ -5666,6 +5666,6 @@ RValue SILGenFunction::emitDynamicSubscriptExpr(DynamicSubscriptExpr *e,
// Package up the result.
auto optResult = optTemp;
if (optTL.isLoadable())
optResult = B.createLoad(e, optResult, LoadOwnershipQualifier::Unqualified);
optResult = optTL.emitLoad(B, e, optResult, LoadOwnershipQualifier::Take);
return RValue(*this, e, emitManagedRValueWithCleanup(optResult, optTL));
}
7 changes: 2 additions & 5 deletions lib/SILGen/SILGenBuiltin.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -607,8 +607,7 @@ emitBuiltinCastReference(SILGenFunction &gen,
return gen.emitManagedBufferWithCleanup(toAddr);

// Load the destination value.
auto result =
gen.B.createLoad(loc, toAddr, LoadOwnershipQualifier::Unqualified);
auto result = toTL.emitLoad(gen.B, loc, toAddr, LoadOwnershipQualifier::Take);
return gen.emitManagedRValueWithCleanup(result);
}

Expand Down Expand Up @@ -643,9 +642,7 @@ static ManagedValue emitBuiltinReinterpretCast(SILGenFunction &gen,
// Load and retain the destination value if it's loadable. Leave the cleanup
// on the original value since we don't know anything about it's type.
if (toTL.isLoadable()) {
SILValue val =
gen.B.createLoad(loc, toAddr, LoadOwnershipQualifier::Unqualified);
return gen.emitManagedRetain(loc, val, toTL);
return gen.emitManagedLoadCopy(loc, toAddr, toTL);
}
// Leave the cleanup on the original value.
if (toTL.isTrivial())
Expand Down
29 changes: 17 additions & 12 deletions lib/SILGen/SILGenConstructor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -275,11 +275,8 @@ void SILGenFunction::emitValueConstructor(ConstructorDecl *ctor) {

if (!lowering.isAddressOnly()) {
// Otherwise, load and return the final 'self' value.
selfValue =
B.createLoad(cleanupLoc, selfLV, LoadOwnershipQualifier::Unqualified);

// Emit a retain of the loaded value, since we return it +1.
selfValue = lowering.emitCopyValue(B, cleanupLoc, selfValue);
selfValue = lowering.emitLoad(B, cleanupLoc, selfLV,
LoadOwnershipQualifier::Copy);

// Inject the self value into an optional if the constructor is failable.
if (ctor->getFailability() != OTK_None) {
Expand Down Expand Up @@ -582,8 +579,9 @@ void SILGenFunction::emitClassConstructorInitializer(ConstructorDecl *ctor) {
if (NeedsBoxForSelf) {
SILLocation prologueLoc = RegularLocation(ctor);
prologueLoc.markAsPrologue();
B.createStore(prologueLoc, selfArg, VarLocs[selfDecl].value,
StoreOwnershipQualifier::Unqualified);
// SEMANTIC ARC TODO: When the verifier is complete, review this.
B.emitStoreValueOperation(prologueLoc, selfArg, VarLocs[selfDecl].value,
StoreOwnershipQualifier::Init);
} else {
selfArg = B.createMarkUninitialized(selfDecl, selfArg, MUKind);
VarLocs[selfDecl] = VarLoc::get(selfArg);
Expand Down Expand Up @@ -670,12 +668,19 @@ void SILGenFunction::emitClassConstructorInitializer(ConstructorDecl *ctor) {
if (Expr *SI = ctor->getSuperInitCall())
emitRValue(SI);

selfArg = B.createLoad(cleanupLoc, VarLocs[selfDecl].value,
LoadOwnershipQualifier::Unqualified);
selfArg = B.emitLoadValueOperation(cleanupLoc, VarLocs[selfDecl].value,
LoadOwnershipQualifier::Copy);
} else {
// We have to do a retain because we are returning the pointer +1.
//
// SEMANTIC ARC TODO: When the verifier is complete, we will need to
// change this to selfArg = B.emitCopyValueOperation(...). Currently due
// to the way that SILGen performs folding of copy_value, destroy_value,
// the returned selfArg may be deleted causing us to have a
// dead-pointer. Instead just use the old self value since we have a
// class.
selfArg = B.emitCopyValueOperation(cleanupLoc, selfArg);
}

// We have to do a retain because we are returning the pointer +1.
selfArg = B.emitCopyValueOperation(cleanupLoc, selfArg);

// Inject the self value into an optional if the constructor is failable.
if (ctor->getFailability() != OTK_None) {
Expand Down
2 changes: 1 addition & 1 deletion lib/SILGen/SILGenConvert.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -250,7 +250,7 @@ ManagedValue SILGenFunction::emitUncheckedGetOptionalValueFrom(SILLocation loc,

if (optTL.isLoadable())
payloadVal =
B.createLoad(loc, payloadVal, LoadOwnershipQualifier::Unqualified);
optTL.emitLoad(B, loc, payloadVal, LoadOwnershipQualifier::Take);
}

// Produce a correctly managed value.
Expand Down
Loading