Skip to content

Commit 3c38c79

Browse files
committed
[region-isolation] Implement MergeIsolationRegionInst.
I am adding this instruction to express artificially that two non-Sendable values should be part of the same region. It is meant to be used in cases where due to unsafe code using Sendable, we stop propagating a non-Sendable dependency that needs to be made in the same region of a use of said Sendable value. I included an example in ./docs/SIL.rst of where this comes up with @out results of continuations.
1 parent dddfdc8 commit 3c38c79

26 files changed

+301
-4
lines changed

SwiftCompilerSources/Sources/SIL/Instruction.swift

+3
Original file line numberDiff line numberDiff line change
@@ -1612,3 +1612,6 @@ final public class CheckedCastAddrBranchInst : TermInst {
16121612

16131613
final public class ThunkInst : Instruction {
16141614
}
1615+
1616+
final public class MergeIsolationRegionInst : Instruction {
1617+
}

SwiftCompilerSources/Sources/SIL/Registration.swift

+1
Original file line numberDiff line numberDiff line change
@@ -257,4 +257,5 @@ public func registerSILClasses() {
257257
register(CheckedCastBranchInst.self)
258258
register(CheckedCastAddrBranchInst.self)
259259
register(ThunkInst.self)
260+
register(MergeIsolationRegionInst.self)
260261
}

docs/SIL.rst

+85-1
Original file line numberDiff line numberDiff line change
@@ -4126,6 +4126,91 @@ pipeline.
41264126

41274127
The operand is a guaranteed operand, i.e. not consumed.
41284128

4129+
merge_isolation_region
4130+
``````````````````````
4131+
4132+
::
4133+
4134+
sil-instruction :: 'merge_isolation_region' (sil-operand ',')+ sil-operand
4135+
4136+
%2 = merge_isolation_region %first : $*T, %second : $U
4137+
%2 = merge_isolation_region %first : $*T, %second : $U, %third : $H
4138+
4139+
Instruction that is only valid in Ownership SSA.
4140+
4141+
This instruction informs region isolation that all of the operands should be
4142+
considered to be artificially apart of the same region. It is intended to be
4143+
used to express region dependency when due to unsafe codegen we have to traffic
4144+
a non-Sendable value through computations with Sendable values (causing us to
4145+
not track the non-Sendable value) but have to later express that a non-Sendable
4146+
result of using the Sendable value needs to be in the same region as the
4147+
original non-Sendable value. As an example of where this comes up, consider the
4148+
following code::
4149+
4150+
// objc code
4151+
@interface CallbackData : NSObject
4152+
@end
4153+
4154+
@interface Klass : NSObject
4155+
4156+
- (void)loadDataWithCompletionHandler:(void (^)(CallbackData * _Nullable, NSError * _Nullable))completionHandler;
4157+
4158+
@end
4159+
4160+
// swift code
4161+
extension Klass {
4162+
func loadCallbackData() async throws -> sending CallbackData {
4163+
try await loadData()
4164+
}
4165+
}
4166+
4167+
This lowers to::
4168+
4169+
%5 = alloc_stack $CallbackData // users: %26, %25, %31, %16, %7
4170+
%6 = objc_method %0 : $Klass, #Klass.loadData!foreign : (Klass) -> () async throws -> CallbackData, $@convention(objc_method) (Optional<@convention(block) (Optional<CallbackData>, Optional<NSError>) -> ()>, Klass) -> () // user: %20
4171+
%7 = get_async_continuation_addr [throws] CallbackData, %5 : $*CallbackData // users: %23, %8
4172+
%8 = struct $UnsafeContinuation<CallbackData, any Error> (%7 : $Builtin.RawUnsafeContinuation) // user: %14
4173+
%9 = alloc_stack $@block_storage Any // users: %22, %16, %10
4174+
%10 = project_block_storage %9 : $*@block_storage Any // user: %11
4175+
%11 = init_existential_addr %10 : $*Any, $CheckedContinuation<CallbackData, any Error> // user: %15
4176+
// function_ref _createCheckedThrowingContinuation<A>(_:)
4177+
%12 = function_ref @$ss34_createCheckedThrowingContinuationyScCyxs5Error_pGSccyxsAB_pGnlF : $@convention(thin) <τ_0_0> (UnsafeContinuation<τ_0_0, any Error>) -> @out CheckedContinuation<τ_0_0, any Error> // user: %14
4178+
%13 = alloc_stack $CheckedContinuation<CallbackData, any Error> // users: %21, %15, %14
4179+
%14 = apply %12<CallbackData>(%13, %8) : $@convention(thin) <τ_0_0> (UnsafeContinuation<τ_0_0, any Error>) -> @out CheckedContinuation<τ_0_0, any Error>
4180+
copy_addr [take] %13 to [init] %11 : $*CheckedContinuation<CallbackData, any Error> // id: %15
4181+
merge_isolation_region %9 : $*@block_storage Any, %5 : $*CallbackData // id: %16
4182+
// function_ref @objc completion handler block implementation for @escaping @callee_unowned @convention(block) (@unowned CallbackData?, @unowned NSError?) -> () with result type CallbackData
4183+
%17 = function_ref @$sSo12CallbackDataCSgSo7NSErrorCSgIeyByy_ABTz_ : $@convention(c) (@inout_aliasable @block_storage Any, Optional<CallbackData>, Optional<NSError>) -> () // user: %18
4184+
%18 = init_block_storage_header %9 : $*@block_storage Any, invoke %17 : $@convention(c) (@inout_aliasable @block_storage Any, Optional<CallbackData>, Optional<NSError>) -> (), type $@convention(block) (Optional<CallbackData>, Optional<NSError>) -> () // user: %19
4185+
%19 = enum $Optional<@convention(block) (Optional<CallbackData>, Optional<NSError>) -> ()>, #Optional.some!enumelt, %18 : $@convention(block) (Optional<CallbackData>, Optional<NSError>) -> () // user: %20
4186+
%20 = apply %6(%19, %0) : $@convention(objc_method) (Optional<@convention(block) (Optional<CallbackData>, Optional<NSError>) -> ()>, Klass) -> ()
4187+
4188+
Notice how without the `merge_isolation_region`_ instruction (%16) there is no
4189+
non-Sendable def-use chain from %5, the indirect return value of the block, to
4190+
the actual non-Sendable block storage %9. This can result in region isolation
4191+
not propagating restrictions on usage from %9 onto %5 risking the creation of
4192+
races.
4193+
4194+
Applying the previous discussion to this specific example, self (%0) is
4195+
non-Sendable and is bound to the current task. If we did not have the
4196+
`merge_isolation_region`_ instruction here, we would not tie the return value %5
4197+
to %0 via %9. This would cause %5 to be treated as a disconnected value and thus
4198+
be a valid sending return value potentially allowing for %5 in the caller of the
4199+
function to be sent to another isolation domain and introduce a race.
4200+
4201+
.. note::
4202+
This is effectively the same purpose that `mark_dependence`_ plays for memory
4203+
dependence (expressing memory dependence that the compiler cannot infer)
4204+
except in the world of region isolation. We purposely use a different
4205+
instruction since `mark_dependence`_ is often times used to create a
4206+
temporary dependence in between two values via the return value of
4207+
`mark_dependence`_. If `mark_dependence`_ had the semantics of acting like a
4208+
region merge we would in contrast have from that point on a region dependence
4209+
in between the base and value of the `mark_dependence`_ causing the
4210+
`mark_dependence`_ to have a less "local" effect since all paths through that
4211+
program point would have to maintain that region dependence until the end of
4212+
the function.
4213+
41294214
dealloc_stack
41304215
`````````````
41314216
::
@@ -9049,7 +9134,6 @@ NOTE: From the perspective of the address checker, a trivial `load`_ with a
90499134
`moveonlywrapper_to_copyable_addr`_ operand is considered to be a use of a
90509135
noncopyable type.
90519136

9052-
90539137
Assertion configuration
90549138
~~~~~~~~~~~~~~~~~~~~~~~
90559139

include/swift/SIL/AddressWalker.h

+2-1
Original file line numberDiff line numberDiff line change
@@ -215,7 +215,8 @@ TransitiveAddressWalker<Impl>::walk(SILValue projectedAddress) && {
215215
isa<RetainValueAddrInst>(user) || isa<ReleaseValueAddrInst>(user) ||
216216
isa<PackElementSetInst>(user) || isa<PackElementGetInst>(user) ||
217217
isa<DeinitExistentialAddrInst>(user) || isa<LoadBorrowInst>(user) ||
218-
isa<TupleAddrConstructorInst>(user) || isa<DeallocPackInst>(user)) {
218+
isa<TupleAddrConstructorInst>(user) || isa<DeallocPackInst>(user) ||
219+
isa<MergeIsolationRegionInst>(user)) {
219220
callVisitUse(op);
220221
continue;
221222
}

include/swift/SIL/SILBuilder.h

+6
Original file line numberDiff line numberDiff line change
@@ -2579,6 +2579,12 @@ class SILBuilder {
25792579
getSILDebugLocation(loc), fnValue, resultType));
25802580
}
25812581

2582+
MergeIsolationRegionInst *
2583+
createMergeIsolationRegion(SILLocation loc, ArrayRef<SILValue> args) {
2584+
return insert(MergeIsolationRegionInst::create(getSILDebugLocation(loc),
2585+
args, getModule()));
2586+
}
2587+
25822588
//===--------------------------------------------------------------------===//
25832589
// Terminator SILInstruction Creation Methods
25842590
//===--------------------------------------------------------------------===//

include/swift/SIL/SILCloner.h

+9
Original file line numberDiff line numberDiff line change
@@ -1168,6 +1168,15 @@ SILCloner<ImplClass>::visitBuiltinInst(BuiltinInst *Inst) {
11681168
getOpSubstitutionMap(Inst->getSubstitutions()), Args));
11691169
}
11701170

1171+
template <typename ImplClass>
1172+
void SILCloner<ImplClass>::visitMergeIsolationRegionInst(
1173+
MergeIsolationRegionInst *Inst) {
1174+
auto Args = getOpValueArray<8>(Inst->getOperandValues());
1175+
getBuilder().setCurrentDebugScope(getOpScope(Inst->getDebugScope()));
1176+
recordClonedInstruction(Inst, getBuilder().createMergeIsolationRegion(
1177+
getOpLocation(Inst->getLoc()), Args));
1178+
}
1179+
11711180
template<typename ImplClass>
11721181
void
11731182
SILCloner<ImplClass>::visitApplyInst(ApplyInst *Inst) {

include/swift/SIL/SILInstruction.h

+19
Original file line numberDiff line numberDiff line change
@@ -11666,6 +11666,25 @@ class TypeValueInst final
1166611666
}
1166711667
};
1166811668

11669+
class MergeIsolationRegionInst final
11670+
: public InstructionBaseWithTrailingOperands<
11671+
SILInstructionKind::MergeIsolationRegionInst,
11672+
MergeIsolationRegionInst, NonValueInstruction> {
11673+
friend SILBuilder;
11674+
11675+
MergeIsolationRegionInst(SILDebugLocation loc, ArrayRef<SILValue> operands)
11676+
: InstructionBaseWithTrailingOperands(operands, loc) {}
11677+
11678+
static MergeIsolationRegionInst *
11679+
create(SILDebugLocation loc, ArrayRef<SILValue> operands, SILModule &mod);
11680+
11681+
public:
11682+
/// Return the SILValues for all operands of this instruction.
11683+
OperandValueArrayRef getArguments() const {
11684+
return OperandValueArrayRef(getAllOperands());
11685+
}
11686+
};
11687+
1166911688
inline SILType *AllocRefInstBase::getTypeStorage() {
1167011689
// If the size of the subclasses are equal, then all of this compiles away.
1167111690
if (auto I = dyn_cast<AllocRefInst>(this))

include/swift/SIL/SILNode.h

+1
Original file line numberDiff line numberDiff line change
@@ -317,6 +317,7 @@ class alignas(8) SILNode :
317317
SHARED_FIELD(SILFunctionArgument, uint32_t noImplicitCopy : 1,
318318
lifetimeAnnotation : 2, closureCapture : 1,
319319
parameterPack : 1);
320+
SHARED_FIELD(MergeRegionIsolationInst, uint32_t numOperands);
320321

321322
// Do not use `_sharedUInt32_private` outside of SILNode.
322323
} _sharedUInt32_private;

include/swift/SIL/SILNodes.def

+3
Original file line numberDiff line numberDiff line change
@@ -894,6 +894,9 @@ NON_VALUE_INST(CondFailInst, cond_fail,
894894
NON_VALUE_INST(MarkUnresolvedMoveAddrInst, mark_unresolved_move_addr,
895895
SILInstruction, None, DoesNotRelease)
896896

897+
NON_VALUE_INST(MergeIsolationRegionInst, merge_isolation_region,
898+
SILInstruction, None, DoesNotRelease)
899+
897900
NON_VALUE_INST(IncrementProfilerCounterInst, increment_profiler_counter,
898901
SILInstruction, MayReadWrite, DoesNotRelease)
899902

lib/IRGen/IRGenSIL.cpp

+5
Original file line numberDiff line numberDiff line change
@@ -1283,6 +1283,11 @@ class IRGenSILFunction :
12831283
auto e = getLoweredExplosion(i->getOperand());
12841284
setLoweredExplosion(i, e);
12851285
}
1286+
1287+
void visitMergeIsolationRegionInst(MergeIsolationRegionInst *i) {
1288+
llvm_unreachable("Valid only when ownership is enabled");
1289+
}
1290+
12861291
void visitReleaseValueInst(ReleaseValueInst *i);
12871292
void visitReleaseValueAddrInst(ReleaseValueAddrInst *i);
12881293
void visitDestroyValueInst(DestroyValueInst *i);

lib/SIL/IR/OperandOwnership.cpp

+1
Original file line numberDiff line numberDiff line change
@@ -228,6 +228,7 @@ OPERAND_OWNERSHIP(InstantaneousUse, ClassifyBridgeObject)
228228
OPERAND_OWNERSHIP(InstantaneousUse, UnownedCopyValue)
229229
OPERAND_OWNERSHIP(InstantaneousUse, WeakCopyValue)
230230
OPERAND_OWNERSHIP(InstantaneousUse, ExtendLifetime)
231+
OPERAND_OWNERSHIP(InstantaneousUse, MergeIsolationRegion)
231232
#define REF_STORAGE(Name, ...) \
232233
OPERAND_OWNERSHIP(InstantaneousUse, StrongCopy##Name##Value)
233234
#include "swift/AST/ReferenceStorage.def"

lib/SIL/IR/SILInstructions.cpp

+8
Original file line numberDiff line numberDiff line change
@@ -3552,3 +3552,11 @@ TypeValueInst *TypeValueInst::create(SILFunction &F, SILDebugLocation loc,
35523552
return ::new (buffer)
35533553
TypeValueInst(loc, typeDependentOperands, valueType, paramType);
35543554
}
3555+
3556+
MergeIsolationRegionInst *
3557+
MergeIsolationRegionInst::create(SILDebugLocation loc, ArrayRef<SILValue> args,
3558+
SILModule &mod) {
3559+
auto size = totalSizeToAlloc<swift::Operand>(args.size());
3560+
auto buffer = mod.allocateInst(size, alignof(MergeIsolationRegionInst));
3561+
return ::new (buffer) MergeIsolationRegionInst(loc, args);
3562+
}

lib/SIL/IR/SILPrinter.cpp

+7-1
Original file line numberDiff line numberDiff line change
@@ -1658,7 +1658,13 @@ class SILPrinter : public SILInstructionVisitor<SILPrinter> {
16581658
*this << ") : ";
16591659
*this << BI->getType();
16601660
}
1661-
1661+
1662+
void visitMergeIsolationRegionInst(MergeIsolationRegionInst *mir) {
1663+
llvm::interleave(
1664+
mir->getArguments(), [&](SILValue v) { *this << getIDAndType(v); },
1665+
[&] { *this << ", "; });
1666+
}
1667+
16621668
void visitAllocGlobalInst(AllocGlobalInst *AGI) {
16631669
if (AGI->getReferencedGlobal()) {
16641670
AGI->getReferencedGlobal()->printName(PrintState.OS);

lib/SIL/Parser/ParseSIL.cpp

+15
Original file line numberDiff line numberDiff line change
@@ -3030,6 +3030,21 @@ bool SILParser::parseSpecificSILInstruction(SILBuilder &B,
30303030
ResultVal = B.createBuiltin(InstLoc, Id, ResultTy, subMap, Args);
30313031
break;
30323032
}
3033+
case SILInstructionKind::MergeIsolationRegionInst: {
3034+
SmallVector<SILValue, 4> Args;
3035+
do {
3036+
SILValue Val;
3037+
if (parseTypedValueRef(Val, B))
3038+
return true;
3039+
Args.push_back(Val);
3040+
} while (P.consumeIf(tok::comma));
3041+
3042+
if (parseSILDebugLocation(InstLoc, B))
3043+
return true;
3044+
3045+
ResultVal = B.createMergeIsolationRegion(InstLoc, Args);
3046+
break;
3047+
}
30333048
case SILInstructionKind::OpenExistentialAddrInst:
30343049
if (parseOpenExistAddrKind() || parseTypedValueRef(Val, B) ||
30353050
parseVerbatim("to") || parseSILType(Ty) ||

lib/SIL/Utils/InstructionUtils.cpp

+1
Original file line numberDiff line numberDiff line change
@@ -517,6 +517,7 @@ RuntimeEffect swift::getRuntimeEffect(SILInstruction *inst, SILType &impactType)
517517
case SILInstructionKind::ClassifyBridgeObjectInst:
518518
case SILInstructionKind::ValueToBridgeObjectInst:
519519
case SILInstructionKind::MarkDependenceInst:
520+
case SILInstructionKind::MergeIsolationRegionInst:
520521
case SILInstructionKind::MoveValueInst:
521522
case SILInstructionKind::DropDeinitInst:
522523
case SILInstructionKind::MarkUnresolvedNonCopyableValueInst:

lib/SIL/Verifier/SILVerifier.cpp

+5
Original file line numberDiff line numberDiff line change
@@ -2187,6 +2187,11 @@ class SILVerifier : public SILVerifierBase<SILVerifier> {
21872187
}
21882188
}
21892189

2190+
void checkMergeIsolationRegionInst(MergeIsolationRegionInst *mir) {
2191+
require(mir->getNumOperands() >= 2, "Must have at least two parameters");
2192+
require(F.hasOwnership(), "Only valid when OSSA is enabled");
2193+
}
2194+
21902195
void checkMarkDependencInst(MarkDependenceInst *MDI) {
21912196
require(isa<SILUndef>(MDI->getValue()) || MDI->getValue() != MDI->getBase(),
21922197
"mark_dependence operands must be distinct");

lib/SILOptimizer/Analysis/RegionAnalysis.cpp

+26
Original file line numberDiff line numberDiff line change
@@ -2232,6 +2232,25 @@ class PartitionOpTranslator {
22322232
requireOperands);
22332233
}
22342234

2235+
void translateSILMerge(MutableArrayRef<Operand> array,
2236+
bool requireOperands = true) {
2237+
if (array.size() < 2)
2238+
return;
2239+
2240+
auto trackableDest = tryToTrackValue(array.front().get());
2241+
if (!trackableDest)
2242+
return;
2243+
for (Operand &op : array.drop_front()) {
2244+
if (auto trackableSrc = tryToTrackValue(op.get())) {
2245+
if (requireOperands) {
2246+
builder.addRequire(trackableSrc->getRepresentative().getValue());
2247+
builder.addRequire(trackableDest->getRepresentative().getValue());
2248+
}
2249+
builder.addMerge(trackableDest->getRepresentative().getValue(), &op);
2250+
}
2251+
}
2252+
}
2253+
22352254
void translateSILAssignmentToTransferringParameter(TrackableValue destRoot,
22362255
Operand *destOperand,
22372256
TrackableValue srcRoot,
@@ -3176,6 +3195,13 @@ PartitionOpTranslator::visitMarkDependenceInst(MarkDependenceInst *mdi) {
31763195
return TranslationSemantics::Special;
31773196
}
31783197

3198+
TranslationSemantics PartitionOpTranslator::visitMergeIsolationRegionInst(
3199+
MergeIsolationRegionInst *mir) {
3200+
// But we want to require and merge the base into the result.
3201+
translateSILMerge(mir->getAllOperands(), true /*require operands*/);
3202+
return TranslationSemantics::Special;
3203+
}
3204+
31793205
TranslationSemantics
31803206
PartitionOpTranslator::visitPointerToAddressInst(PointerToAddressInst *ptai) {
31813207
if (!isNonSendableType(ptai->getType())) {

lib/SILOptimizer/Mandatory/OwnershipModelEliminator.cpp

+5
Original file line numberDiff line numberDiff line change
@@ -151,6 +151,11 @@ struct OwnershipModelEliminatorVisitor
151151
eraseInstruction(dbi);
152152
return true;
153153
}
154+
bool visitMergeIsolationRegionInst(MergeIsolationRegionInst *mir) {
155+
eraseInstruction(mir);
156+
return true;
157+
}
158+
154159
bool visitLoadBorrowInst(LoadBorrowInst *lbi);
155160
bool visitMoveValueInst(MoveValueInst *mvi) {
156161
eraseInstructionAndRAUW(mvi, mvi->getOperand());

lib/SILOptimizer/UtilityPasses/SerializeSILPass.cpp

+1
Original file line numberDiff line numberDiff line change
@@ -167,6 +167,7 @@ static bool hasOpaqueArchetype(TypeExpansionContext context,
167167
case SILInstructionKind::ClassifyBridgeObjectInst:
168168
case SILInstructionKind::ValueToBridgeObjectInst:
169169
case SILInstructionKind::MarkDependenceInst:
170+
case SILInstructionKind::MergeIsolationRegionInst:
170171
case SILInstructionKind::CopyBlockInst:
171172
case SILInstructionKind::CopyBlockWithoutEscapingInst:
172173
case SILInstructionKind::CopyValueInst:

lib/SILOptimizer/Utils/SILInliner.cpp

+1
Original file line numberDiff line numberDiff line change
@@ -877,6 +877,7 @@ InlineCost swift::instructionInlineCost(SILInstruction &I) {
877877
case SILInstructionKind::BeginBorrowInst:
878878
case SILInstructionKind::BorrowedFromInst:
879879
case SILInstructionKind::MarkDependenceInst:
880+
case SILInstructionKind::MergeIsolationRegionInst:
880881
case SILInstructionKind::PreviousDynamicFunctionRefInst:
881882
case SILInstructionKind::DynamicFunctionRefInst:
882883
case SILInstructionKind::FunctionRefInst:

0 commit comments

Comments
 (0)