Skip to content

[6.1][rbi] Change Region Based Isolation for closures to not use the AST and instead just use SIL. #78990

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

Closed
3 changes: 3 additions & 0 deletions SwiftCompilerSources/Sources/SIL/Instruction.swift
Original file line number Diff line number Diff line change
Expand Up @@ -1615,3 +1615,6 @@ final public class ThunkInst : Instruction {

final public class MergeIsolationRegionInst : Instruction {
}

final public class IgnoredUseInst : Instruction, UnaryInstruction {
}
1 change: 1 addition & 0 deletions SwiftCompilerSources/Sources/SIL/Registration.swift
Original file line number Diff line number Diff line change
Expand Up @@ -258,4 +258,5 @@ public func registerSILClasses() {
register(CheckedCastAddrBranchInst.self)
register(ThunkInst.self)
register(MergeIsolationRegionInst.self)
register(IgnoredUseInst.self)
}
6 changes: 6 additions & 0 deletions include/swift/AST/DiagnosticsSIL.def
Original file line number Diff line number Diff line change
Expand Up @@ -1054,6 +1054,12 @@ NOTE(regionbasedisolation_typed_tns_passed_to_sending_closure_helper_have_value_
NOTE(regionbasedisolation_typed_tns_passed_to_sending_closure_helper_multiple_value, none,
"closure captures non-Sendable %0",
(DeclName))
NOTE(regionbasedisolation_closure_captures, none,
"closure captures %0",
(DeclName))
NOTE(regionbasedisolation_closure_captures_actor, none,
"closure captures %0 allowing access to %1 state within the closure",
(DeclName, StringRef))

NOTE(regionbasedisolation_typed_tns_passed_to_sending_callee, none,
"Passing %0 value of non-Sendable type %1 as a 'sending' parameter to %2 %3 risks causing races inbetween %0 uses and uses reachable from %3",
Expand Down
5 changes: 5 additions & 0 deletions include/swift/SIL/InstructionUtils.h
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,11 @@ bool isEndOfScopeMarker(SILInstruction *user);
/// only used in recognizable patterns without otherwise "escaping".
bool isIncidentalUse(SILInstruction *user);

/// Returns true if this is a move only wrapper use.
///
/// E.x.: moveonlywrapper_to_copyable_addr, copyable_to_moveonlywrapper_value
bool isMoveOnlyWrapperUse(SILInstruction *user);

/// Return true if the given `user` instruction modifies the value's refcount
/// without propagating the value or having any other effect aside from
/// potentially destroying the value itself (and executing associated cleanups).
Expand Down
9 changes: 9 additions & 0 deletions include/swift/SIL/SILBuilder.h
Original file line number Diff line number Diff line change
Expand Up @@ -3089,6 +3089,15 @@ class SILBuilder {
getModule(), getSILDebugLocation(Loc), Decl));
}

//===--------------------------------------------------------------------===//
// Misc Uses
//===--------------------------------------------------------------------===//

IgnoredUseInst *createIgnoredUse(SILLocation loc, SILValue value) {
return insert(new (getModule())
IgnoredUseInst(getSILDebugLocation(loc), value));
}

//===--------------------------------------------------------------------===//
// Private Helper Methods
//===--------------------------------------------------------------------===//
Expand Down
8 changes: 8 additions & 0 deletions include/swift/SIL/SILCloner.h
Original file line number Diff line number Diff line change
Expand Up @@ -3796,6 +3796,14 @@ void SILCloner<ImplClass>::visitHasSymbolInst(HasSymbolInst *Inst) {
Inst->getDecl()));
}

template <typename ImplClass>
void SILCloner<ImplClass>::visitIgnoredUseInst(IgnoredUseInst *Inst) {
getBuilder().setCurrentDebugScope(getOpScope(Inst->getDebugScope()));
recordClonedInstruction(
Inst, getBuilder().createIgnoredUse(getOpLocation(Inst->getLoc()),
getOpValue(Inst->getOperand())));
}

} // end namespace swift

#endif
11 changes: 11 additions & 0 deletions include/swift/SIL/SILInstruction.h
Original file line number Diff line number Diff line change
Expand Up @@ -11685,6 +11685,17 @@ class MergeIsolationRegionInst final
}
};

/// An instruction that represents a semantic-less use that is used to
/// suppresses unused value variable warnings. E.x.: _ = x.
class IgnoredUseInst final
: public UnaryInstructionBase<SILInstructionKind::IgnoredUseInst,
NonValueInstruction> {
friend SILBuilder;

IgnoredUseInst(SILDebugLocation loc, SILValue operand)
: UnaryInstructionBase(loc, operand) {}
};

inline SILType *AllocRefInstBase::getTypeStorage() {
// If the size of the subclasses are equal, then all of this compiles away.
if (auto I = dyn_cast<AllocRefInst>(this))
Expand Down
3 changes: 3 additions & 0 deletions include/swift/SIL/SILNodes.def
Original file line number Diff line number Diff line change
Expand Up @@ -897,6 +897,9 @@ NON_VALUE_INST(MarkUnresolvedMoveAddrInst, mark_unresolved_move_addr,
NON_VALUE_INST(MergeIsolationRegionInst, merge_isolation_region,
SILInstruction, None, DoesNotRelease)

NON_VALUE_INST(IgnoredUseInst, ignored_use,
SILInstruction, None, DoesNotRelease)

NON_VALUE_INST(IncrementProfilerCounterInst, increment_profiler_counter,
SILInstruction, MayReadWrite, DoesNotRelease)

Expand Down
6 changes: 6 additions & 0 deletions include/swift/SILOptimizer/Utils/PartitionUtils.h
Original file line number Diff line number Diff line change
Expand Up @@ -1292,6 +1292,12 @@ struct PartitionOpEvaluator {
Region sentRegion = p.getRegion(sentElement);
bool isClosureCapturedElt = false;
SILDynamicMergedIsolationInfo sentRegionIsolation;

// TODO: Today we only return the first element in our region that has
// some form of isolation. This causes us to in the case of sending
// partial_applies to only emit a diagnostic for the first element in the
// capture list of the partial_apply. If we returned a list of potential
// errors... we could emit the error for each capture individually.
auto pairOpt = getIsolationRegionInfo(sentRegion, op.getSourceOp());
if (!pairOpt) {
return handleError(UnknownCodePatternError(op));
Expand Down
3 changes: 3 additions & 0 deletions lib/IRGen/IRGenSIL.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1273,6 +1273,9 @@ class IRGenSILFunction :
visitMoveOnlyWrapperToCopyableBoxInst(MoveOnlyWrapperToCopyableBoxInst *i) {
llvm_unreachable("OSSA instruction");
}

void visitIgnoredUseInst(IgnoredUseInst *i) {}

void
visitMoveOnlyWrapperToCopyableAddrInst(MoveOnlyWrapperToCopyableAddrInst *i) {
auto e = getLoweredExplosion(i->getOperand());
Expand Down
1 change: 1 addition & 0 deletions lib/SIL/IR/OperandOwnership.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -214,6 +214,7 @@ OPERAND_OWNERSHIP(TrivialUse, GlobalAddr)
// The dealloc_stack_ref operand needs to have NonUse ownership because
// this use comes after the last consuming use (which is usually a dealloc_ref).
OPERAND_OWNERSHIP(NonUse, DeallocStackRef)
OPERAND_OWNERSHIP(InstantaneousUse, IgnoredUse)

// Use an owned or guaranteed value only for the duration of the operation.
OPERAND_OWNERSHIP(InstantaneousUse, ExistentialMetatype)
Expand Down
4 changes: 4 additions & 0 deletions lib/SIL/IR/SILPrinter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2812,6 +2812,10 @@ class SILPrinter : public SILInstructionVisitor<SILPrinter> {
*this << GI->getFormalResumeType();
}

void visitIgnoredUseInst(IgnoredUseInst *i) {
*this << getIDAndType(i->getOperand());
}

void visitGetAsyncContinuationAddrInst(GetAsyncContinuationAddrInst *GI) {
if (GI->throws())
*this << "[throws] ";
Expand Down
5 changes: 5 additions & 0 deletions lib/SIL/Parser/ParseSIL.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5068,6 +5068,11 @@ bool SILParser::parseSpecificSILInstruction(SILBuilder &B,
}
break;
}
case SILInstructionKind::IgnoredUseInst:
if (parseTypedValueRef(Val, B) || parseSILDebugLocation(InstLoc, B))
return true;
ResultVal = B.createIgnoredUse(InstLoc, Val);
break;

case SILInstructionKind::DeallocStackInst:
if (parseTypedValueRef(Val, B) || parseSILDebugLocation(InstLoc, B))
Expand Down
17 changes: 16 additions & 1 deletion lib/SIL/Utils/InstructionUtils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -334,7 +334,21 @@ bool swift::isEndOfScopeMarker(SILInstruction *user) {

bool swift::isIncidentalUse(SILInstruction *user) {
return isEndOfScopeMarker(user) || user->isDebugInstruction() ||
isa<FixLifetimeInst>(user) || isa<EndLifetimeInst>(user);
isa<FixLifetimeInst>(user) || isa<EndLifetimeInst>(user) ||
isa<IgnoredUseInst>(user);
}

bool swift::isMoveOnlyWrapperUse(SILInstruction *user) {
switch (user->getKind()) {
case SILInstructionKind::MoveOnlyWrapperToCopyableValueInst:
case SILInstructionKind::MoveOnlyWrapperToCopyableBoxInst:
case SILInstructionKind::MoveOnlyWrapperToCopyableAddrInst:
case SILInstructionKind::CopyableToMoveOnlyWrapperValueInst:
case SILInstructionKind::CopyableToMoveOnlyWrapperAddrInst:
return true;
default:
return false;
}
}

bool swift::onlyAffectsRefCount(SILInstruction *user) {
Expand Down Expand Up @@ -622,6 +636,7 @@ RuntimeEffect swift::getRuntimeEffect(SILInstruction *inst, SILType &impactType)
case SILInstructionKind::DebugStepInst:
case SILInstructionKind::FunctionExtractIsolationInst:
case SILInstructionKind::TypeValueInst:
case SILInstructionKind::IgnoredUseInst:
return RuntimeEffect::NoEffect;

case SILInstructionKind::OpenExistentialMetatypeInst:
Expand Down
1 change: 1 addition & 0 deletions lib/SIL/Verifier/SILVerifier.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -719,6 +719,7 @@ struct ImmutableAddressUseVerifier {
case SILInstructionKind::KeyPathInst:
case SILInstructionKind::SwitchEnumAddrInst:
case SILInstructionKind::SelectEnumAddrInst:
case SILInstructionKind::IgnoredUseInst:
break;
case SILInstructionKind::DebugValueInst:
if (cast<DebugValueInst>(inst)->hasAddrVal())
Expand Down
71 changes: 36 additions & 35 deletions lib/SILGen/SILGenDecl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1582,38 +1582,34 @@ void SILGenFunction::emitPatternBinding(PatternBindingDecl *PBD, unsigned idx,
return nullptr;
};

auto emitInitializer = [&](Expr *initExpr, VarDecl *var, bool forLocalContext,
InitializationPtr &initialization) {
// If an initial value expression was specified by the decl, emit it into
// the initialization.
FullExpr Scope(Cleanups, CleanupLocation(initExpr));

if (forLocalContext) {
if (auto *orig = var->getOriginalWrappedProperty()) {
if (auto *initExpr = getWrappedValueExpr(var)) {
auto value = emitRValue(initExpr);
emitApplyOfPropertyWrapperBackingInitializer(
auto *initExpr = PBD->getExecutableInit(idx);

// If we do not have an explicit initialization expression, just mark the
// initialization as unfinished for DI to resolve.
if (!initExpr) {
return initialization->finishUninitialized(*this);
}

// Otherwise, an initial value expression was specified by the decl... emit it
// into the initialization.
FullExpr Scope(Cleanups, CleanupLocation(initExpr));

auto *singleVar = PBD->getSingleVar();
bool isLocalSingleVar =
singleVar && singleVar->getDeclContext()->isLocalContext();
if (isLocalSingleVar) {
if (auto *orig = singleVar->getOriginalWrappedProperty()) {
if (auto *initExpr = getWrappedValueExpr(singleVar)) {
auto value = emitRValue(initExpr);
emitApplyOfPropertyWrapperBackingInitializer(
PBD, orig, getForwardingSubstitutionMap(), std::move(value))
.forwardInto(*this, SILLocation(PBD), initialization.get());
return;
}
return;
}
}

emitExprInto(initExpr, initialization.get());
};

auto *singleVar = PBD->getSingleVar();
if (auto *Init = PBD->getExecutableInit(idx)) {
// If an initial value expression was specified by the decl, emit it into
// the initialization.
bool isLocalVar =
singleVar && singleVar->getDeclContext()->isLocalContext();
emitInitializer(Init, singleVar, isLocalVar, initialization);
} else {
// Otherwise, mark it uninitialized for DI to resolve.
initialization->finishUninitialized(*this);
}

emitExprInto(initExpr, initialization.get());
}

void SILGenFunction::visitPatternBindingDecl(PatternBindingDecl *PBD,
Expand Down Expand Up @@ -2421,18 +2417,23 @@ void BlackHoleInitialization::performPackExpansionInitialization(

void BlackHoleInitialization::copyOrInitValueInto(SILGenFunction &SGF, SILLocation loc,
ManagedValue value, bool isInit) {
// Normally we do not do anything if we have a black hole
// initialization... but if we have a move only object, insert a move value.
if (!value.getType().isMoveOnly())
// If we do not have a noncopyable type, just insert an ignored use.
if (!value.getType().isMoveOnly()) {
SGF.B.createIgnoredUse(loc, value.getValue());
return;
}

// If we have an address, then this will create a new temporary allocation
// which will trigger the move checker. If we have an object though, we need
// to insert an extra move_value to make sure the object checker behaves
// correctly.
// If we have a noncopyable type, we need to do a little more work to satisfy
// the move checkers. If we have an address, then this will create a new
// temporary allocation which will trigger the move checker...
value = value.ensurePlusOne(SGF, loc);
if (value.getType().isAddress())
if (value.getType().isAddress()) {
SGF.B.createIgnoredUse(loc, value.getValue());
return;
}

// If we have an object though, we need to insert an extra move_value to make
// sure the object checker behaves correctly.
value = SGF.B.createMoveValue(loc, value);
SGF.B.createIgnoredUse(loc, value.getValue());
}
31 changes: 30 additions & 1 deletion lib/SILGen/SILGenExpr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5307,7 +5307,19 @@ static void emitSimpleAssignment(SILGenFunction &SGF, SILLocation loc,
value = value.ensurePlusOne(SGF, loc);
if (value.getType().isObject())
value = SGF.B.createMoveValue(loc, value);
SGF.B.createIgnoredUse(loc, value.getValue());
return;
}

// Emit the ignored use instruction like we would do in emitIgnoredExpr.
if (!rv.isNull()) {
SmallVector<ManagedValue, 16> values;
std::move(rv).getAll(values);
for (auto v : values) {
SGF.B.createIgnoredUse(loc, v.getValue());
}
}

return;
}

Expand Down Expand Up @@ -6958,7 +6970,24 @@ void SILGenFunction::emitIgnoredExpr(Expr *E) {

// Otherwise, emit the result (to get any side effects), but produce it at +0
// if that allows simplification.
emitRValue(E, SGFContext::AllowImmediatePlusZero);
RValue rv = emitRValue(E, SGFContext::AllowImmediatePlusZero);

// Return early if we do not have any result.
if (rv.isNull())
return;

// Then emit ignored use on all of the rvalue components. We purposely do not
// implode the tuple since if we have a tuple formation like:
//
// let _ = (x, y)
//
// we want the use to be on x and y and not on the imploded tuple. It also
// helps us avoid emitting unnecessary code.
SmallVector<ManagedValue, 16> values;
std::move(rv).getAll(values);
for (auto v : values) {
B.createIgnoredUse(E, v.getValue());
}
}

/// Emit the given expression as an r-value, then (if it is a tuple), combine
Expand Down
1 change: 1 addition & 0 deletions lib/SILOptimizer/Analysis/RegionAnalysis.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3053,6 +3053,7 @@ CONSTANT_TRANSLATION(DebugStepInst, Ignored)
CONSTANT_TRANSLATION(IncrementProfilerCounterInst, Ignored)
CONSTANT_TRANSLATION(SpecifyTestInst, Ignored)
CONSTANT_TRANSLATION(TypeValueInst, Ignored)
CONSTANT_TRANSLATION(IgnoredUseInst, Ignored)

//===---
// Require
Expand Down
9 changes: 9 additions & 0 deletions lib/SILOptimizer/Mandatory/DiagnoseUnreachable.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -775,6 +775,15 @@ static bool simplifyBlocksWithCallsToNoReturn(SILBasicBlock &BB,
if (isa<EndBorrowInst>(currInst))
return false;

// If we have an ignored use whose operand is our no return call, ignore it.
if (auto *i = dyn_cast<IgnoredUseInst>(currInst)) {
// This handles try_apply, apply, begin_apply.
if (auto *inst = i->getOperand()->getDefiningInstructionOrTerminator();
inst && inst == noReturnCall) {
return false;
}
}

// destroy_value [dead_end] instructions are inserted at the availability
// boundary by lifetime completion. Such instructions correctly mark the
// lifetime boundary of the destroyed value and never arise from dead user
Expand Down
Loading