Skip to content

Commit 8e21bfc

Browse files
jckarterjckarter
committed
MoveOnlyAddressChecker: Confine analysis to current formal access.
Code can only locally interact with a mutable memory location within a formal access, and is only responsible for maintaining its invariants during that access, so the move-only address checker does not need to, and should not, observe operations that occur outside of the access marked with the `mark_must_check` instruction. And for immutable memory locations, although there are no explicit formal accesses, that's because every access must be read-only, so although individual accesses are not delimited, they are all compatible as far as move-only checking is concerned. So we can back out the changes to SILGen to re-project a memory location from its origin on every access, a change which breaks invariants assumed by other SIL passes.
1 parent e5ea789 commit 8e21bfc

16 files changed

+222
-251
lines changed

Diff for: ‎lib/SILGen/SILGenConstructor.cpp

-6
Original file line numberDiff line numberDiff line change
@@ -558,8 +558,6 @@ void SILGenFunction::emitValueConstructor(ConstructorDecl *ctor) {
558558

559559
ManagedValue selfLV =
560560
maybeEmitValueOfLocalVarDecl(selfDecl, AccessKind::ReadWrite);
561-
if (!selfLV)
562-
selfLV = maybeEmitAddressForBoxOfLocalVarDecl(selfDecl, selfDecl);
563561
assert(selfLV);
564562

565563
// Emit the prolog.
@@ -1223,10 +1221,6 @@ static ManagedValue emitSelfForMemberInit(SILGenFunction &SGF, SILLocation loc,
12231221
SGFContext::AllowImmediatePlusZero)
12241222
.getAsSingleValue(SGF, loc);
12251223
} else {
1226-
// First see if we have a variable that is boxed without a value.
1227-
if (auto value = SGF.maybeEmitAddressForBoxOfLocalVarDecl(loc, selfDecl))
1228-
return value;
1229-
// Otherwise, emit the address directly.
12301224
return SGF.emitAddressOfLocalVarDecl(loc, selfDecl, selfFormalType,
12311225
SGFAccessKind::Write);
12321226
}

Diff for: ‎lib/SILGen/SILGenDecl.cpp

+3-8
Original file line numberDiff line numberDiff line change
@@ -539,8 +539,7 @@ class LocalVariableInitialization : public SingleBufferInitialization {
539539
}
540540
}
541541

542-
if (!Box->getType().isBoxedNonCopyableType(Box->getFunction()))
543-
Addr = SGF.B.createProjectBox(decl, Box, 0);
542+
Addr = SGF.B.createProjectBox(decl, Box, 0);
544543

545544
// Push a cleanup to destroy the local variable. This has to be
546545
// inactive until the variable is initialized.
@@ -589,10 +588,7 @@ class LocalVariableInitialization : public SingleBufferInitialization {
589588
/// decl to.
590589
assert(SGF.VarLocs.count(decl) == 0 && "Already emitted the local?");
591590

592-
if (Addr)
593-
SGF.VarLocs[decl] = SILGenFunction::VarLoc::get(Addr, Box);
594-
else
595-
SGF.VarLocs[decl] = SILGenFunction::VarLoc::getForBox(Box);
591+
SGF.VarLocs[decl] = SILGenFunction::VarLoc::get(Addr, Box);
596592

597593
SingleBufferInitialization::finishInitialization(SGF);
598594
assert(!DidFinish &&
@@ -657,8 +653,7 @@ class LetValueInitialization : public Initialization {
657653
// If this is a let with an initializer or bound value, we only need a
658654
// buffer if the type is address only or is noncopyable.
659655
//
660-
// For noncopyable types, we always need to box them and eagerly
661-
// reproject.
656+
// For noncopyable types, we always need to box them.
662657
needsTemporaryBuffer =
663658
(lowering->isAddressOnly() && SGF.silConv.useLoweredAddresses()) ||
664659
lowering->getLoweredType().isPureMoveOnly();

Diff for: ‎lib/SILGen/SILGenFunction.cpp

+2-2
Original file line numberDiff line numberDiff line change
@@ -358,7 +358,7 @@ void SILGenFunction::emitCaptures(SILLocation loc,
358358
// Get an address value for a SILValue if it is address only in an type
359359
// expansion context without opaque archetype substitution.
360360
auto getAddressValue = [&](VarLoc entryVarLoc) -> SILValue {
361-
SILValue entryValue = entryVarLoc.getValueOrBoxedValue(*this, vd);
361+
SILValue entryValue = entryVarLoc.value;
362362
if (SGM.M.useLoweredAddresses()
363363
&& SGM.Types
364364
.getTypeLowering(
@@ -383,7 +383,7 @@ void SILGenFunction::emitCaptures(SILLocation loc,
383383
case CaptureKind::Constant: {
384384
// let declarations.
385385
auto &tl = getTypeLowering(valueType);
386-
SILValue Val = Entry.getValueOrBoxedValue(*this);
386+
SILValue Val = Entry.value;
387387
bool eliminateMoveOnlyWrapper =
388388
Val->getType().isMoveOnlyWrapped() &&
389389
!vd->getInterfaceType()->is<SILMoveOnlyWrappedType>();

Diff for: ‎lib/SILGen/SILGenFunction.h

-22
Original file line numberDiff line numberDiff line change
@@ -436,25 +436,6 @@ class LLVM_LIBRARY_VISIBILITY SILGenFunction
436436
Result.box = box;
437437
return Result;
438438
}
439-
440-
static VarLoc getForBox(SILValue box) {
441-
VarLoc Result;
442-
Result.value = SILValue();
443-
Result.box = box;
444-
return Result;
445-
}
446-
447-
/// Return either the value if we have one or if we only have a box, project
448-
/// our a new box address and return that.
449-
SILValue getValueOrBoxedValue(SILGenFunction &SGF,
450-
SILLocation loc = SILLocation::invalid()) {
451-
if (value)
452-
return value;
453-
assert(box);
454-
if (loc.isNull())
455-
loc = SGF.CurrentSILLoc;
456-
return SGF.B.createProjectBox(loc, box, 0);
457-
}
458439
};
459440

460441
/// VarLocs - Entries in this map are generated when a PatternBindingDecl is
@@ -1541,9 +1522,6 @@ class LLVM_LIBRARY_VISIBILITY SILGenFunction
15411522
ManagedValue maybeEmitValueOfLocalVarDecl(
15421523
VarDecl *var, AccessKind accessKind);
15431524

1544-
ManagedValue maybeEmitAddressForBoxOfLocalVarDecl(SILLocation loc,
1545-
VarDecl *var);
1546-
15471525
/// Produce an RValue for a reference to the specified declaration,
15481526
/// with the given type and in response to the specified expression. Try to
15491527
/// emit into the specified SGFContext to avoid copies (when provided).

Diff for: ‎lib/SILGen/SILGenLValue.cpp

+22-48
Original file line numberDiff line numberDiff line change
@@ -1090,18 +1090,27 @@ namespace {
10901090
ManagedValue base) && override {
10911091
assert(!base && "value component must be root of lvalue path");
10921092

1093-
// See if we have a noncopyable address from a project_box or global, we
1094-
// always eagerly reproject out.
1093+
// See if we have a noncopyable address from a project_box or global.
10951094
if (Value.getType().isAddress() && Value.getType().isMoveOnly()) {
10961095
SILValue addr = Value.getValue();
1097-
if (isa<ProjectBoxInst>(addr) || isa<GlobalAddrInst>(addr)) {
1096+
auto box = dyn_cast<ProjectBoxInst>(addr);
1097+
if (box || isa<GlobalAddrInst>(addr)) {
10981098
if (Enforcement)
10991099
addr = enterAccessScope(SGF, loc, base, addr, getTypeData(),
11001100
getAccessKind(), *Enforcement,
11011101
takeActorIsolation());
1102-
addr = SGF.B.createMarkMustCheckInst(
1103-
loc, addr,
1104-
MarkMustCheckInst::CheckKind::AssignableButNotConsumable);
1102+
// LValue accesses to a `let` box are only ever going to make through
1103+
// definite initialization if they are initializations, which don't
1104+
// require checking since there's no former value to potentially
1105+
// misuse yet.
1106+
if (!box || box->getOperand()->getType().castTo<SILBoxType>()
1107+
->getLayout()->isMutable()) {
1108+
addr = SGF.B.createMarkMustCheckInst(
1109+
loc, addr,
1110+
isReadAccess(getAccessKind())
1111+
? MarkMustCheckInst::CheckKind::NoConsumeOrAssign
1112+
: MarkMustCheckInst::CheckKind::AssignableButNotConsumable);
1113+
}
11051114
return ManagedValue::forLValue(addr);
11061115
}
11071116
}
@@ -3009,11 +3018,8 @@ void LValue::addNonMemberVarComponent(SILGenFunction &SGF, SILLocation loc,
30093018
auto astAccessKind = mapAccessKind(this->AccessKind);
30103019
auto address = SGF.maybeEmitValueOfLocalVarDecl(Storage, astAccessKind);
30113020

3012-
// The only other case that should get here is a global variable or a
3013-
// noncopyable value in a box.
3021+
// The only other case that should get here is a global variable.
30143022
if (!address) {
3015-
address = SGF.maybeEmitAddressForBoxOfLocalVarDecl(Loc, Storage);
3016-
if (!address)
30173023
address = SGF.emitGlobalVariableRef(Loc, Storage, ActorIso);
30183024
} else {
30193025
assert((!ActorIso || Storage->isTopLevelGlobal()) &&
@@ -3056,26 +3062,6 @@ void LValue::addNonMemberVarComponent(SILGenFunction &SGF, SILLocation loc,
30563062
emitter.emitUsingStrategy(strategy);
30573063
}
30583064

3059-
ManagedValue
3060-
SILGenFunction::maybeEmitAddressForBoxOfLocalVarDecl(SILLocation loc,
3061-
VarDecl *var) {
3062-
auto It = VarLocs.find(var);
3063-
3064-
// Wasn't a box.
3065-
if (It == VarLocs.end())
3066-
return ManagedValue();
3067-
3068-
SILValue value = It->second.value;
3069-
SILValue box = It->second.box;
3070-
3071-
// We only want to do this if we only have a box without a value.
3072-
if (!box || value)
3073-
return ManagedValue();
3074-
3075-
SILValue addr = B.createProjectBox(loc, box, 0);
3076-
return ManagedValue::forLValue(addr);
3077-
}
3078-
30793065
ManagedValue
30803066
SILGenFunction::maybeEmitValueOfLocalVarDecl(
30813067
VarDecl *var, AccessKind accessKind) {
@@ -3084,13 +3070,7 @@ SILGenFunction::maybeEmitValueOfLocalVarDecl(
30843070
if (It != VarLocs.end()) {
30853071
SILValue ptr = It->second.value;
30863072

3087-
// If we do not have an actual value stored, then we must have a box.
3088-
if (!ptr) {
3089-
assert(It->second.box);
3090-
return ManagedValue();
3091-
}
3092-
3093-
// If this has an address, return it. By-value let's have no address.
3073+
// If this has an address, return it. By-value let bindings have no address.
30943074
if (ptr->getType().isAddress())
30953075
return ManagedValue::forLValue(ptr);
30963076

@@ -3101,7 +3081,7 @@ SILGenFunction::maybeEmitValueOfLocalVarDecl(
31013081
return ManagedValue::forUnmanaged(ptr);
31023082
}
31033083

3104-
// Otherwise, it's non-local, not stored, or a box.
3084+
// Otherwise, it's non-local or not stored.
31053085
return ManagedValue();
31063086
}
31073087

@@ -3116,9 +3096,6 @@ SILGenFunction::emitAddressOfLocalVarDecl(SILLocation loc, VarDecl *var,
31163096
assert(!var->isAsyncLet() && "async let does not have an address");
31173097

31183098
auto address = maybeEmitValueOfLocalVarDecl(var, astAccessKind);
3119-
if (!address)
3120-
address = maybeEmitAddressForBoxOfLocalVarDecl(loc, var);
3121-
assert(address);
31223099
assert(address.isLValue());
31233100
return address;
31243101
}
@@ -3141,12 +3118,7 @@ RValue SILGenFunction::emitRValueForNonMemberVarDecl(SILLocation loc,
31413118

31423119
// If our localValue is a closure captured box of a noncopyable type, project
31433120
// it out eagerly and insert a no_consume_or_assign constraint.
3144-
ManagedValue localValue;
3145-
if (auto localAddr = maybeEmitAddressForBoxOfLocalVarDecl(loc, var)) {
3146-
localValue = localAddr;
3147-
} else {
3148-
localValue = maybeEmitValueOfLocalVarDecl(var, AccessKind::Read);
3149-
}
3121+
ManagedValue localValue = maybeEmitValueOfLocalVarDecl(var, AccessKind::Read);
31503122

31513123
// If this VarDecl is represented as an address, emit it as an lvalue, then
31523124
// perform a load to get the rvalue.
@@ -3169,9 +3141,11 @@ RValue SILGenFunction::emitRValueForNonMemberVarDecl(SILLocation loc,
31693141
SILAccessKind::Read);
31703142

31713143
if (accessAddr->getType().isMoveOnly()) {
3144+
// When loading an rvalue, we should never need to modify the place
3145+
// we're loading from.
31723146
accessAddr = B.createMarkMustCheckInst(
31733147
loc, accessAddr,
3174-
MarkMustCheckInst::CheckKind::AssignableButNotConsumable);
3148+
MarkMustCheckInst::CheckKind::NoConsumeOrAssign);
31753149
}
31763150

31773151
auto propagateRValuePastAccess = [&](RValue &&rvalue) {

Diff for: ‎lib/SILGen/SILGenPattern.cpp

+5-5
Original file line numberDiff line numberDiff line change
@@ -2708,7 +2708,7 @@ static void switchCaseStmtSuccessCallback(SILGenFunction &SGF,
27082708
// Emit a debug description for the variable, nested within a scope
27092709
// for the pattern match.
27102710
SILDebugVariable dbgVar(vd->isLet(), /*ArgNo=*/0);
2711-
SGF.B.emitDebugDescription(vd, v.getValueOrBoxedValue(SGF), dbgVar);
2711+
SGF.B.emitDebugDescription(vd, v.value, dbgVar);
27122712
}
27132713
}
27142714
}
@@ -2748,7 +2748,7 @@ static void switchCaseStmtSuccessCallback(SILGenFunction &SGF,
27482748
if (!var->hasName() || var->getName() != expected->getName())
27492749
continue;
27502750

2751-
SILValue value = SGF.VarLocs[var].getValueOrBoxedValue(SGF);
2751+
SILValue value = SGF.VarLocs[var].value;
27522752
SILType type = value->getType();
27532753

27542754
// If we have an address-only type, initialize the temporary
@@ -3008,7 +3008,7 @@ void SILGenFunction::emitSwitchFallthrough(FallthroughStmt *S) {
30083008
}
30093009

30103010
auto varLoc = VarLocs[var];
3011-
SILValue value = varLoc.getValueOrBoxedValue(*this);
3011+
SILValue value = varLoc.value;
30123012

30133013
if (value->getType().isAddressOnly(F)) {
30143014
context->Emission.emitAddressOnlyInitialization(expected, value);
@@ -3074,7 +3074,7 @@ void SILGenFunction::emitCatchDispatch(DoCatchStmt *S, ManagedValue exn,
30743074
// Emit a debug description of the incoming arg, nested within the scope
30753075
// for the pattern match.
30763076
SILDebugVariable dbgVar(vd->isLet(), /*ArgNo=*/0);
3077-
B.emitDebugDescription(vd, v.getValueOrBoxedValue(*this), dbgVar);
3077+
B.emitDebugDescription(vd, v.value, dbgVar);
30783078
}
30793079
}
30803080
}
@@ -3121,7 +3121,7 @@ void SILGenFunction::emitCatchDispatch(DoCatchStmt *S, ManagedValue exn,
31213121
if (!var->hasName() || var->getName() != expected->getName())
31223122
continue;
31233123

3124-
SILValue value = VarLocs[var].getValueOrBoxedValue(*this);
3124+
SILValue value = VarLocs[var].value;
31253125
SILType type = value->getType();
31263126

31273127
// If we have an address-only type, initialize the temporary

Diff for: ‎lib/SILGen/SILGenProlog.cpp

+6-11
Original file line numberDiff line numberDiff line change
@@ -674,10 +674,9 @@ class ArgumentInitHelper {
674674
SGF.emitManagedRValueWithCleanup(box);
675675

676676
// We manually set calledCompletedUpdate to true since we want to use
677-
// VarLoc::getForBox and use the debug info from the box rather than
678-
// insert a custom debug_value.
677+
// the debug info from the box rather than insert a custom debug_value.
679678
calledCompletedUpdate = true;
680-
SGF.VarLocs[pd] = SILGenFunction::VarLoc::getForBox(box);
679+
SGF.VarLocs[pd] = SILGenFunction::VarLoc::get(destAddr, box);
681680
return;
682681
}
683682

@@ -947,14 +946,10 @@ static void emitCaptureArguments(SILGenFunction &SGF,
947946
auto *box = SGF.F.begin()->createFunctionArgument(
948947
SILType::getPrimitiveObjectType(boxTy), VD);
949948
box->setClosureCapture(true);
950-
if (box->getType().getSILBoxFieldType(&SGF.F, 0).isMoveOnly()) {
951-
SGF.VarLocs[VD] = SILGenFunction::VarLoc::getForBox(box);
952-
} else {
953-
SILValue addr = SGF.B.createProjectBox(VD, box, 0);
954-
SGF.VarLocs[VD] = SILGenFunction::VarLoc::get(addr, box);
955-
SILDebugVariable DbgVar(VD->isLet(), ArgNo);
956-
SGF.B.createDebugValueAddr(Loc, addr, DbgVar);
957-
}
949+
SILValue addr = SGF.B.createProjectBox(VD, box, 0);
950+
SGF.VarLocs[VD] = SILGenFunction::VarLoc::get(addr, box);
951+
SILDebugVariable DbgVar(VD->isLet(), ArgNo);
952+
SGF.B.createDebugValueAddr(Loc, addr, DbgVar);
958953
break;
959954
}
960955
case CaptureKind::Immutable:

Diff for: ‎lib/SILOptimizer/Mandatory/DefiniteInitialization.cpp

+1-2
Original file line numberDiff line numberDiff line change
@@ -2337,8 +2337,7 @@ void LifetimeChecker::updateInstructionForInitState(unsigned UseID) {
23372337
if (auto *mmci =
23382338
dyn_cast<MarkMustCheckInst>(stripAccessMarkers(AI->getDest()))) {
23392339
if (mmci->getCheckKind() ==
2340-
MarkMustCheckInst::CheckKind::AssignableButNotConsumable &&
2341-
isa<RefElementAddrInst>(stripAccessMarkers(mmci->getOperand()))) {
2340+
MarkMustCheckInst::CheckKind::AssignableButNotConsumable) {
23422341
mmci->setCheckKind(
23432342
MarkMustCheckInst::CheckKind::InitableButNotConsumable);
23442343
}

0 commit comments

Comments
 (0)