Skip to content

[5.9] More batched changes #66196

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 13 commits into from
Jun 5, 2023
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
9 changes: 4 additions & 5 deletions include/swift/AST/DiagnosticsSIL.def
Original file line number Diff line number Diff line change
Expand Up @@ -758,11 +758,10 @@ ERROR(sil_movechecking_value_used_after_consume, none,
"'%0' used after consume", (StringRef))
ERROR(sil_movechecking_guaranteed_value_consumed, none,
"'%0' is borrowed and cannot be consumed", (StringRef))

// FIXME: this diagnostic shouldn't ever be emitted now. rdar://109742587 (closures may still try to consume captures, e.g., borrowed parameters)
ERROR(sil_movechecking_guaranteed_value_captured_by_closure, none,
"'%0' is borrowed and cannot be consumed by closure capture", (StringRef))

ERROR(sil_movechecking_borrowed_parameter_captured_by_closure, none,
"'%0' cannot be captured by an escaping closure since it is a borrowed "
"parameter",
(StringRef))
ERROR(sil_movechecking_capture_consumed, none,
"noncopyable '%0' cannot be consumed when captured by a closure", (StringRef))
ERROR(sil_movechecking_inout_not_reinitialized_before_end_of_function, none,
Expand Down
5 changes: 5 additions & 0 deletions include/swift/SIL/SILFunctionConventions.h
Original file line number Diff line number Diff line change
Expand Up @@ -371,6 +371,11 @@ class SILFunctionConventions {
return getNumIndirectSILResults();
}

/// Returns the index of self.
unsigned getSILArgIndexOfSelf() const {
return getSILArgIndexOfFirstParam() + getNumParameters() - 1;
}

/// Get the index into formal indirect results corresponding to the given SIL
/// indirect result argument index.
unsigned getIndirectFormalResultIndexForSILArg(unsigned argIdx) const {
Expand Down
7 changes: 6 additions & 1 deletion include/swift/SIL/SILMoveOnlyDeinit.h
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,12 @@ class SILMoveOnlyDeinit final : public SILAllocated<SILMoveOnlyDeinit> {

SILFunction *getImplementation() const { return funcImpl; }

bool isSerialized() const { return serialized; }
IsSerialized_t isSerialized() const {
return serialized ? IsSerialized : IsNotSerialized;
}
void setSerialized(IsSerialized_t inputSerialized) {
serialized = inputSerialized ? 1 : 0;
}

void print(llvm::raw_ostream &os, bool verbose) const;
void dump() const;
Expand Down
3 changes: 1 addition & 2 deletions include/swift/SILOptimizer/Utils/CanonicalizeOSSALifetime.h
Original file line number Diff line number Diff line change
Expand Up @@ -319,10 +319,9 @@ class CanonicalizeOSSALifetime final {
liveness->initializeDef(getCurrentDef());
}

void invalidateLiveness() {
void clear() {
consumingBlocks.clear();
debugValues.clear();
liveness->invalidate();
discoveredBlocks.clear();
}

Expand Down
19 changes: 13 additions & 6 deletions lib/IRGen/GenType.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2887,19 +2887,26 @@ static bool tryEmitDeinitCall(IRGenFunction &IGF,
llvm::function_ref<void ()> indirectCleanup) {
auto ty = T.getASTType();
auto nominal = ty->getAnyNominal();

// We are only concerned with move-only type deinits here.
if (!nominal || !nominal->getValueTypeDestructor()) {
return false;
}

auto deinit = IGF.getSILModule().lookUpMoveOnlyDeinit(nominal);
assert(deinit && "type has a deinit declared in AST but SIL deinit record is not present!");


auto deinitTable = IGF.getSILModule().lookUpMoveOnlyDeinit(nominal);

// If we do not have a deinit table, call the value witness instead.
if (!deinitTable) {
irgen::emitDestroyCall(IGF, T, indirect());
Copy link
Contributor

Choose a reason for hiding this comment

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

You need to call indirectCleanup after the call to indirect.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will prepare a small patch on main that will fix this. Why don't you give the +1 and then I can get TimK to +1 this and I can take care of this tonight/over the weekend.

indirectCleanup();
return true;
}

// The deinit should take a single value parameter of the nominal type, either
// by @owned or indirect @in convention.
auto deinitFn = IGF.IGM.getAddrOfSILFunction(deinit->getImplementation(),
auto deinitFn = IGF.IGM.getAddrOfSILFunction(deinitTable->getImplementation(),
NotForDefinition);
auto deinitTy = deinit->getImplementation()->getLoweredFunctionType();
auto deinitTy = deinitTable->getImplementation()->getLoweredFunctionType();
auto deinitFP = FunctionPointer::forDirect(IGF.IGM, deinitFn,
nullptr, deinitTy);
assert(deinitTy->getNumParameters() == 1
Expand Down
2 changes: 1 addition & 1 deletion lib/SILGen/ArgumentSource.h
Original file line number Diff line number Diff line change
Expand Up @@ -280,7 +280,7 @@ class ArgumentSource {

ArgumentSource copyForDiagnostics() const;

void dump() const;
LLVM_DUMP_METHOD void dump() const;
void dump(raw_ostream &os, unsigned indent = 0) const;

private:
Expand Down
5 changes: 0 additions & 5 deletions lib/SILGen/SILGen.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1079,11 +1079,6 @@ void SILGenModule::emitFunctionDefinition(SILDeclRef constant, SILFunction *f) {
preEmitFunction(constant, f, loc);
PrettyStackTraceSILFunction X("silgen emitDeallocatingDestructor", f);
SILGenFunction(*this, *f, dd).emitDeallocatingDestructor(dd);

// If we have a move only type, create the table for this type.
if (nom->isMoveOnly())
SILMoveOnlyDeinit::create(f->getModule(), nom, IsNotSerialized, f);

postEmitFunction(constant, f);
return;
}
Expand Down
3 changes: 3 additions & 0 deletions lib/SILGen/SILGen.h
Original file line number Diff line number Diff line change
Expand Up @@ -459,6 +459,9 @@ class LLVM_LIBRARY_VISIBILITY SILGenModule : public ASTVisitor<SILGenModule> {
SILFunction *jvp, SILFunction *vjp,
const DeclAttribute *diffAttr);

/// Emit a deinit table for a noncopyable type.
void emitNonCopyableTypeDeinitTable(NominalTypeDecl *decl);

/// Known functions for bridging.
SILDeclRef getStringToNSStringFn();
SILDeclRef getNSStringToStringFn();
Expand Down
27 changes: 26 additions & 1 deletion lib/SILGen/SILGenApply.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3029,7 +3029,8 @@ static StorageRefResult findStorageReferenceExprForBorrow(Expr *e) {
return result.withTransitiveRoot(te);

} else if (auto ioe = dyn_cast<InOutExpr>(e)) {
return ioe;
if (auto result = findStorageReferenceExprForBorrow(ioe->getSubExpr()))
return result.withTransitiveRoot(ioe);
}

return StorageRefResult();
Expand All @@ -3049,6 +3050,24 @@ Expr *ArgumentSource::findStorageReferenceExprForMoveOnly(
sawLoad = true;
}

// If we have a subscript, strip it off and make sure that our base is
// something that we can process. If we do and we succeed below, we return the
// subscript instead.
SubscriptExpr *subscriptExpr = nullptr;
if ((subscriptExpr = dyn_cast<SubscriptExpr>(argExpr))) {
auto *decl = cast<SubscriptDecl>(subscriptExpr->getDecl().getDecl());
if (decl->getReadImpl() != ReadImplKind::Read) {
subscriptExpr = nullptr;
} else {
argExpr = subscriptExpr->getBase();
}

// If there's a load on the base of the subscript expr, look past it.
if (auto *li = dyn_cast<LoadExpr>(argExpr)) {
argExpr = li->getSubExpr();
}
}

// If we're consuming instead, then the load _must_ have been there.
if (kind == StorageReferenceOperationKind::Consume && !sawLoad)
return nullptr;
Expand Down Expand Up @@ -3097,6 +3116,12 @@ Expr *ArgumentSource::findStorageReferenceExprForMoveOnly(
// has a move only base.
(void)std::move(*this).asKnownExpr();

// If we saw a subscript expr and the base of the subscript expr passed our
// tests above, we can emit the call to the subscript directly as a borrowed
// lvalue. Return the subscript expr here so that we emit it appropriately.
if (subscriptExpr)
return subscriptExpr;

return result.getTransitiveRoot();
}

Expand Down
6 changes: 6 additions & 0 deletions lib/SILGen/SILGenDecl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1958,13 +1958,19 @@ InitializationPtr SILGenFunction::emitLocalVariableWithCleanup(
std::unique_ptr<TemporaryInitialization>
SILGenFunction::emitTemporary(SILLocation loc, const TypeLowering &tempTL) {
SILValue addr = emitTemporaryAllocation(loc, tempTL.getLoweredType());
if (addr->getType().isMoveOnly())
addr = B.createMarkMustCheckInst(
loc, addr, MarkMustCheckInst::CheckKind::ConsumableAndAssignable);
return useBufferAsTemporary(addr, tempTL);
}

std::unique_ptr<TemporaryInitialization>
SILGenFunction::emitFormalAccessTemporary(SILLocation loc,
const TypeLowering &tempTL) {
SILValue addr = emitTemporaryAllocation(loc, tempTL.getLoweredType());
if (addr->getType().isMoveOnly())
addr = B.createMarkMustCheckInst(
loc, addr, MarkMustCheckInst::CheckKind::ConsumableAndAssignable);
CleanupHandle cleanup =
enterDormantFormalAccessTemporaryCleanup(addr, loc, tempTL);
return std::unique_ptr<TemporaryInitialization>(
Expand Down
6 changes: 5 additions & 1 deletion lib/SILGen/SILGenLValue.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -366,6 +366,7 @@ LogicalPathComponent::projectForRead(SILGenFunction &SGF, SILLocation loc,
}

TemporaryInitializationPtr tempInit;
ManagedValue result;
RValue rvalue;

// If the RValue type has an openedExistential, then the RValue must be
Expand All @@ -377,9 +378,11 @@ LogicalPathComponent::projectForRead(SILGenFunction &SGF, SILLocation loc,

// Create a temporary, whose type may depend on the 'get'.
tempInit = SGF.emitFormalAccessTemporary(loc, RValueTL);
result = tempInit->getManagedAddress();
} else {
// Create a temporary for a static (non-dependent) RValue type.
tempInit = SGF.emitFormalAccessTemporary(loc, RValueTL);
result = tempInit->getManagedAddress();

// Emit a 'get' directly into the temporary.
rvalue = std::move(*this).get(SGF, loc, base, SGFContext(tempInit.get()));
Expand All @@ -390,7 +393,8 @@ LogicalPathComponent::projectForRead(SILGenFunction &SGF, SILLocation loc,
if (!rvalue.isInContext())
std::move(rvalue).forwardInto(SGF, loc, tempInit.get());

return tempInit->getManagedAddress();
assert(result);
return result;
}

ManagedValue LogicalPathComponent::project(SILGenFunction &SGF,
Expand Down
22 changes: 22 additions & 0 deletions lib/SILGen/SILGenType.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1069,6 +1069,21 @@ void SILGenModule::emitDefaultWitnessTable(ProtocolDecl *protocol) {
defaultWitnesses->convertToDefinition(builder.DefaultWitnesses);
}

void SILGenModule::emitNonCopyableTypeDeinitTable(NominalTypeDecl *nom) {
auto *dd = nom->getValueTypeDestructor();
if (!dd)
return;

SILDeclRef constant(dd, SILDeclRef::Kind::Deallocator);
SILFunction *f = getFunction(constant, NotForDefinition);
auto serialized = IsSerialized_t::IsNotSerialized;
bool nomIsPublic = nom->getEffectiveAccess() >= AccessLevel::Public;
// We only serialize the deinit if the type is public and not resilient.
if (nomIsPublic && !nom->isResilient())
serialized = IsSerialized;
SILMoveOnlyDeinit::create(f->getModule(), nom, serialized, f);
}

namespace {

/// An ASTVisitor for generating SIL from method declarations
Expand Down Expand Up @@ -1100,6 +1115,13 @@ class SILGenType : public TypeMemberVisitor<SILGenType> {
genVTable.emitVTable();
}

// If this is a nominal type that is move only, emit a deinit table for it.
if (auto *nom = dyn_cast<NominalTypeDecl>(theType)) {
if (nom->isMoveOnly()) {
SGM.emitNonCopyableTypeDeinitTable(nom);
}
}

// Build a default witness table if this is a protocol that needs one.
if (auto protocol = dyn_cast<ProtocolDecl>(theType)) {
if (!protocol->isObjC() && protocol->isResilient()) {
Expand Down
42 changes: 20 additions & 22 deletions lib/SILOptimizer/Mandatory/MoveOnlyAddressCheckerUtils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2523,29 +2523,27 @@ void MoveOnlyAddressCheckerPImpl::rewriteUses(
// scope, we would have emitted the scope expansion error during diagnostics.
for (auto pair : addressUseState.borrows) {
if (auto *li = dyn_cast<LoadInst>(pair.first)) {
if (li->getOwnershipQualifier() == LoadOwnershipQualifier::Copy) {
// If we had a load [copy], borrow then we know that all of its destroys
// must have been destroy_value. So we can just gather up those
// destroy_value and use then to create a new load_borrow scope.
SILBuilderWithScope builder(li);
auto *lbi = builder.createLoadBorrow(li->getLoc(), li->getOperand());
// We use this auxillary list to avoid iterator invalidation of
// li->getConsumingUse();
StackList<DestroyValueInst *> toDelete(lbi->getFunction());
for (auto *consumeUse : li->getConsumingUses()) {
auto *dvi = cast<DestroyValueInst>(consumeUse->getUser());
SILBuilderWithScope destroyBuilder(dvi);
destroyBuilder.createEndBorrow(dvi->getLoc(), lbi);
toDelete.push_back(dvi);
changed = true;
}
while (!toDelete.empty())
toDelete.pop_back_val()->eraseFromParent();

li->replaceAllUsesWith(lbi);
li->eraseFromParent();
continue;
// If we had a load -> load_borrow then we know that all of its destroys
// must have been destroy_value. So we can just gather up those
// destroy_value and use then to create a new load_borrow scope.
SILBuilderWithScope builder(li);
auto *lbi = builder.createLoadBorrow(li->getLoc(), li->getOperand());
// We use this auxillary list to avoid iterator invalidation of
// li->getConsumingUse();
StackList<DestroyValueInst *> toDelete(lbi->getFunction());
for (auto *consumeUse : li->getConsumingUses()) {
auto *dvi = cast<DestroyValueInst>(consumeUse->getUser());
SILBuilderWithScope destroyBuilder(dvi);
destroyBuilder.createEndBorrow(dvi->getLoc(), lbi);
toDelete.push_back(dvi);
changed = true;
}
while (!toDelete.empty())
toDelete.pop_back_val()->eraseFromParent();

li->replaceAllUsesWith(lbi);
li->eraseFromParent();
continue;
}

llvm::dbgs() << "Borrow: " << *pair.first;
Expand Down
28 changes: 23 additions & 5 deletions lib/SILOptimizer/Mandatory/MoveOnlyDeinitInsertion.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -77,9 +77,21 @@ static bool performTransform(SILFunction &fn) {
auto subMap =
astType->getContextSubstitutionMap(nom->getModuleContext(), nom);
SILBuilderWithScope builder(dvi);

SILValue value = dvi->getOperand();
auto conv = deinitFunc->getConventionsInContext();
if (conv.getSILArgumentConvention(conv.getSILArgIndexOfSelf())
.isIndirectConvention()) {
auto *asi =
builder.createAllocStack(dvi->getLoc(), value->getType());
builder.emitStoreValueOperation(dvi->getLoc(), value, asi,
StoreOwnershipQualifier::Init);
value = asi;
}
auto *funcRef = builder.createFunctionRef(dvi->getLoc(), deinitFunc);
builder.createApply(dvi->getLoc(), funcRef, subMap,
dvi->getOperand());
builder.createApply(dvi->getLoc(), funcRef, subMap, value);
if (isa<AllocStackInst>(value))
builder.createDeallocStack(dvi->getLoc(), value);
dvi->eraseFromParent();
changed = true;
continue;
Expand All @@ -103,9 +115,15 @@ static bool performTransform(SILFunction &fn) {
auto *funcRef = builder.createFunctionRef(dai->getLoc(), deinitFunc);
auto subMap = destroyType.getASTType()->getContextSubstitutionMap(
nom->getModuleContext(), nom);
auto loadedValue = builder.emitLoadValueOperation(
dai->getLoc(), dai->getOperand(), LoadOwnershipQualifier::Take);
builder.createApply(dai->getLoc(), funcRef, subMap, loadedValue);

auto conv = deinitFunc->getConventionsInContext();
auto argConv =
conv.getSILArgumentConvention(conv.getSILArgIndexOfSelf());
SILValue value = dai->getOperand();
if (!argConv.isIndirectConvention())
value = builder.emitLoadValueOperation(
dai->getLoc(), dai->getOperand(), LoadOwnershipQualifier::Take);
builder.createApply(dai->getLoc(), funcRef, subMap, value);
dai->eraseFromParent();
changed = true;
continue;
Expand Down
2 changes: 1 addition & 1 deletion lib/SILOptimizer/Mandatory/MoveOnlyDiagnostics.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,7 @@ void DiagnosticEmitter::emitObjectGuaranteedDiagnostic(
// See if we have any closure capture uses and emit a better diagnostic.
if (getCanonicalizer().hasPartialApplyConsumingUse()) {
diagnose(astContext, markedValue,
diag::sil_movechecking_guaranteed_value_captured_by_closure,
diag::sil_movechecking_borrowed_parameter_captured_by_closure,
varName);
emitObjectDiagnosticsForPartialApplyUses(varName);
registerDiagnosticEmitted(markedValue);
Expand Down
4 changes: 4 additions & 0 deletions lib/SILOptimizer/UtilityPasses/SerializeSILPass.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -472,6 +472,10 @@ class SerializeSILPass : public SILModuleTransform {
for (auto &VT : M.getVTables()) {
VT->setSerialized(IsNotSerialized);
}

for (auto &Deinit : M.getMoveOnlyDeinits()) {
Deinit->setSerialized(IsNotSerialized);
}
}

public:
Expand Down
4 changes: 2 additions & 2 deletions lib/SILOptimizer/Utils/CanonicalizeOSSALifetime.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1042,7 +1042,7 @@ bool CanonicalizeOSSALifetime::computeLiveness() {
// Step 1: compute liveness
if (!computeCanonicalLiveness()) {
LLVM_DEBUG(llvm::errs() << "Failed to compute canonical liveness?!\n");
invalidateLiveness();
clear();
return false;
}
if (accessBlockAnalysis) {
Expand Down Expand Up @@ -1076,7 +1076,7 @@ void CanonicalizeOSSALifetime::rewriteLifetimes() {
// Step 6: rewrite copies and delete extra destroys
rewriteCopies();

invalidateLiveness();
clear();
consumes.clear();
}

Expand Down
Loading