Skip to content

Commit 662d5fd

Browse files
jckarterjckarter
authored
Merge pull request #14460 from jckarter/open-existential-lvalue-evaluation-order-4.1
[4.1] SILGen: Open existential lvalues on entry into an OpenExistentialExpr again.
2 parents b091e73 + f310e5d commit 662d5fd

File tree

7 files changed

+73
-55
lines changed

7 files changed

+73
-55
lines changed

‎lib/SILGen/SILGenExpr.cpp

Lines changed: 28 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5089,15 +5089,34 @@ RValue RValueEmitter::emitForceValue(ForceValueExpr *loc, Expr *E,
50895089
void SILGenFunction::emitOpenExistentialExprImpl(
50905090
OpenExistentialExpr *E,
50915091
llvm::function_ref<void(Expr *)> emitSubExpr) {
5092-
Optional<FormalEvaluationScope> writebackScope;
5093-
50945092
// Emit the existential value.
50955093
if (E->getExistentialValue()->getType()->is<LValueType>()) {
5096-
bool inserted = OpaqueValueExprs.insert({E->getOpaqueValue(), E}).second;
5094+
// Open the existential container right away. We need the dynamic type
5095+
// to be opened in order to evaluate the subexpression.
5096+
AccessKind accessKind;
5097+
if (E->hasLValueAccessKind())
5098+
accessKind = E->getLValueAccessKind();
5099+
else
5100+
accessKind = E->getExistentialValue()->getLValueAccessKind();
5101+
auto lv = emitLValue(E->getExistentialValue(), accessKind);
5102+
auto formalOpenedType = E->getOpaqueValue()->getType()
5103+
->getWithoutSpecifierType()
5104+
->getCanonicalType();
5105+
lv = emitOpenExistentialLValue(E, std::move(lv),
5106+
CanArchetypeType(E->getOpenedArchetype()),
5107+
formalOpenedType,
5108+
accessKind);
5109+
5110+
auto addr = emitAddressOfLValue(E, std::move(lv), accessKind);
5111+
bool inserted = OpaqueLValues.insert({E->getOpaqueValue(),
5112+
{addr.getValue(), formalOpenedType}})
5113+
.second;
50975114
(void)inserted;
50985115
assert(inserted && "already have this opened existential?");
50995116

51005117
emitSubExpr(E->getSubExpr());
5118+
assert(OpaqueLValues.count(E->getOpaqueValue()) == 0
5119+
&& "opened existential not removed?");
51015120
return;
51025121
}
51035122

@@ -5124,6 +5143,12 @@ RValue RValueEmitter::visitOpenExistentialExpr(OpenExistentialExpr *E,
51245143
return RValue(SGF, E, *result);
51255144
}
51265145

5146+
Optional<FormalEvaluationScope> scope;
5147+
// Begin an evaluation scope for an lvalue existential opened into an
5148+
// rvalue expression.
5149+
if (E->getExistentialValue()->getType()->is<LValueType>())
5150+
scope.emplace(SGF);
5151+
51275152
return SGF.emitOpenExistentialExpr<RValue>(E,
51285153
[&](Expr *subExpr) -> RValue {
51295154
return visit(subExpr, C);

‎lib/SILGen/SILGenFunction.h

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1416,10 +1416,10 @@ class LLVM_LIBRARY_VISIBILITY SILGenFunction
14161416
llvm::SmallDenseMap<OpaqueValueExpr *, OpaqueValueState>
14171417
OpaqueValues;
14181418

1419-
/// A mapping from opaque value expressions to the open-existential
1420-
/// expression that determines them, used while lowering lvalues.
1421-
llvm::SmallDenseMap<OpaqueValueExpr *, OpenExistentialExpr *>
1422-
OpaqueValueExprs;
1419+
/// A mapping from opaque value lvalue expressions to the address of the
1420+
/// opened value in memory.
1421+
llvm::SmallDenseMap<OpaqueValueExpr *, std::pair<SILValue, CanType>>
1422+
OpaqueLValues;
14231423

14241424
/// RAII object that introduces a temporary binding for an opaque value.
14251425
///

‎lib/SILGen/SILGenLValue.cpp

Lines changed: 18 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -2262,20 +2262,18 @@ LValue SILGenLValue::visitOpaqueValueExpr(OpaqueValueExpr *e,
22622262
AccessKind accessKind,
22632263
LValueOptions options) {
22642264
// Handle an opaque lvalue that refers to an opened existential.
2265-
auto known = SGF.OpaqueValueExprs.find(e);
2266-
if (known != SGF.OpaqueValueExprs.end()) {
2267-
// Dig the open-existential expression out of the list.
2268-
OpenExistentialExpr *opened = known->second;
2269-
SGF.OpaqueValueExprs.erase(known);
2270-
2271-
// Do formal evaluation of the underlying existential lvalue.
2272-
auto lv = visitRec(opened->getExistentialValue(), accessKind, options);
2273-
lv = SGF.emitOpenExistentialLValue(
2274-
opened, std::move(lv),
2275-
CanArchetypeType(opened->getOpenedArchetype()),
2276-
e->getType()->getWithoutSpecifierType()->getCanonicalType(),
2277-
accessKind);
2278-
return lv;
2265+
auto known = SGF.OpaqueLValues.find(e);
2266+
if (known != SGF.OpaqueLValues.end()) {
2267+
// Dig the opened address out of the list.
2268+
SILValue opened = known->second.first;
2269+
CanType openedType = known->second.second;
2270+
SGF.OpaqueLValues.erase(known);
2271+
2272+
// Continue formal evaluation of the lvalue from this address.
2273+
return LValue::forAddress(ManagedValue::forLValue(opened),
2274+
None,
2275+
AbstractionPattern(openedType),
2276+
openedType);
22792277
}
22802278

22812279
assert(SGF.OpaqueValues.count(e) && "Didn't bind OpaqueValueExpr");
@@ -2567,29 +2565,12 @@ LValue SILGenLValue::visitTupleElementExpr(TupleElementExpr *e,
25672565
LValue SILGenLValue::visitOpenExistentialExpr(OpenExistentialExpr *e,
25682566
AccessKind accessKind,
25692567
LValueOptions options) {
2570-
// If the opaque value is not an lvalue, open the existential immediately.
2571-
if (!e->getOpaqueValue()->getType()->is<LValueType>()) {
2572-
return SGF.emitOpenExistentialExpr<LValue>(e,
2573-
[&](Expr *subExpr) -> LValue {
2574-
return visitRec(subExpr,
2575-
accessKind,
2576-
options);
2577-
});
2578-
}
2579-
2580-
// Record the fact that we're opening this existential. The actual
2581-
// opening operation will occur when we see the OpaqueValueExpr.
2582-
bool inserted = SGF.OpaqueValueExprs.insert({e->getOpaqueValue(), e}).second;
2583-
(void)inserted;
2584-
assert(inserted && "already have this opened existential?");
2585-
2586-
// Visit the subexpression.
2587-
LValue lv = visitRec(e->getSubExpr(), accessKind, options);
2588-
2589-
// Sanity check that we did see the OpaqueValueExpr.
2590-
assert(SGF.OpaqueValueExprs.count(e->getOpaqueValue()) == 0 &&
2591-
"opened existential not removed?");
2592-
return lv;
2568+
return SGF.emitOpenExistentialExpr<LValue>(e,
2569+
[&](Expr *subExpr) -> LValue {
2570+
return visitRec(subExpr,
2571+
accessKind,
2572+
options);
2573+
});
25932574
}
25942575

25952576
static LValueTypeData

‎test/SILGen/assignment.swift

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -43,11 +43,11 @@ protocol P {
4343
// CHECK-LABEL: sil hidden @_T010assignment15copyRightToLeftyAA1P_pz1p_tF : $@convention(thin) (@inout P) -> () {
4444
func copyRightToLeft(p: inout P) {
4545
// CHECK: bb0(%0 : @trivial $*P):
46+
// CHECK: [[WRITE:%.*]] = begin_access [modify] [unknown] %0 : $*P
47+
// CHECK: [[WRITE_OPEN:%.*]] = open_existential_addr mutable_access [[WRITE]]
4648
// CHECK: [[READ:%.*]] = begin_access [read] [unknown] %0 : $*P
4749
// CHECK: [[READ_OPEN:%.*]] = open_existential_addr immutable_access [[READ]]
4850
// CHECK: end_access [[READ]] : $*P
49-
// CHECK: [[WRITE:%.*]] = begin_access [modify] [unknown] %0 : $*P
50-
// CHECK: [[WRITE_OPEN:%.*]] = open_existential_addr mutable_access [[WRITE]]
5151
// CHECK: end_access [[WRITE]] : $*P
5252
p.left = p.right
5353
}

‎test/SILGen/class_bound_protocols.swift

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -192,23 +192,23 @@ func takesInheritsMutatingMethod(x: inout InheritsMutatingMethod,
192192
// CHECK-NEXT: [[X_PAYLOAD:%.*]] = open_existential_ref [[X_VALUE]] : $InheritsMutatingMethod to $@opened("{{.*}}") InheritsMutatingMethod
193193
// CHECK-NEXT: [[TEMPORARY:%.*]] = alloc_stack $@opened("{{.*}}") InheritsMutatingMethod
194194
// CHECK-NEXT: store [[X_PAYLOAD]] to [init] [[TEMPORARY]] : $*@opened("{{.*}}") InheritsMutatingMethod
195-
// CHECK-NEXT: [[X_PAYLOAD_RELOADED:%.*]] = load [take] [[TEMPORARY]]
195+
// CHECK-NEXT: [[X_PAYLOAD_RELOADED:%.*]] = load_borrow [[TEMPORARY]]
196196
//
197197
// ** *NOTE* This extra copy is here since RValue invariants enforce that all
198198
// ** loadable objects are actually loaded. So we form the RValue and
199199
// ** load... only to then need to store the value back in a stack location to
200200
// ** pass to an in_guaranteed method. PredictableMemOpts is able to handle this
201201
// ** type of temporary codegen successfully.
202202
// CHECK-NEXT: [[TEMPORARY_2:%.*]] = alloc_stack $@opened("{{.*}}") InheritsMutatingMethod
203-
// CHECK-NEXT: store [[X_PAYLOAD_RELOADED:%.*]] to [init] [[TEMPORARY_2]]
203+
// CHECK-NEXT: store_borrow [[X_PAYLOAD_RELOADED:%.*]] to [[TEMPORARY_2]]
204204
//
205205
// CHECK-NEXT: [[METHOD:%.*]] = witness_method $@opened("{{.*}}") InheritsMutatingMethod, #HasMutatingMethod.mutatingCounter!getter.1 : <Self where Self : HasMutatingMethod> (Self) -> () -> Value, [[X_PAYLOAD]] : $@opened("{{.*}}") InheritsMutatingMethod : $@convention(witness_method: HasMutatingMethod) <τ_0_0 where τ_0_0 : HasMutatingMethod> (@in_guaranteed τ_0_0) -> Value
206206
// CHECK-NEXT: [[RESULT_VALUE:%.*]] = apply [[METHOD]]<@opened("{{.*}}") InheritsMutatingMethod>([[TEMPORARY_2]]) : $@convention(witness_method: HasMutatingMethod) <τ_0_0 where τ_0_0 : HasMutatingMethod> (@in_guaranteed τ_0_0) -> Value
207-
// CHECK-NEXT: [[X_VALUE:%.*]] = load [take] [[TEMPORARY_2]] : $*@opened("{{.*}}") InheritsMutatingMethod
208-
// CHECK-NEXT: destroy_value [[X_VALUE]] : $@opened("{{.*}}") InheritsMutatingMethod
209207
// CHECK-NEXT: dealloc_stack [[TEMPORARY_2]]
210-
// CHECK-NEXT: end_access [[X_ADDR]] : $*InheritsMutatingMethod
208+
// CHECK-NEXT: end_borrow [[X_PAYLOAD_RELOADED]]
211209
// CHECK-NEXT: assign [[RESULT_VALUE]] to [[RESULT]] : $*Value
210+
// CHECK-NEXT: destroy_addr [[TEMPORARY]]
211+
// CHECK-NEXT: end_access [[X_ADDR]]
212212
// CHECK-NEXT: dealloc_stack [[TEMPORARY]] : $*@opened("{{.*}}") InheritsMutatingMethod
213213
// CHECK-NEXT: dealloc_stack [[RESULT_BOX]] : $*Value
214214
_ = x.mutatingCounter

‎test/SILGen/properties.swift

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1190,11 +1190,11 @@ protocol NonmutatingProtocol {
11901190
// CHECK-NEXT: [[C_FIELD_PAYLOAD:%.*]] = open_existential_addr immutable_access [[C_FIELD_BOX]] : $*NonmutatingProtocol to $*@opened("{{.*}}") NonmutatingProtocol
11911191
// CHECK-NEXT: [[C_FIELD_COPY:%.*]] = alloc_stack $@opened("{{.*}}") NonmutatingProtocol
11921192
// CHECK-NEXT: copy_addr [[C_FIELD_PAYLOAD]] to [initialization] [[C_FIELD_COPY]] : $*@opened("{{.*}}") NonmutatingProtocol
1193-
// CHECK-NEXT: destroy_addr [[C_FIELD_BOX]] : $*NonmutatingProtocol
11941193
// CHECK-NEXT: [[GETTER:%.*]] = witness_method $@opened("{{.*}}") NonmutatingProtocol, #NonmutatingProtocol.x!getter.1 : <Self where Self : NonmutatingProtocol> (Self) -> () -> Int, [[C_FIELD_PAYLOAD]] : $*@opened("{{.*}}") NonmutatingProtocol : $@convention(witness_method: NonmutatingProtocol) <τ_0_0 where τ_0_0 : NonmutatingProtocol> (@in_guaranteed τ_0_0) -> Int
11951194
// CHECK-NEXT: [[RESULT_VALUE:%.*]] = apply [[GETTER]]<@opened("{{.*}}") NonmutatingProtocol>([[C_FIELD_COPY]]) : $@convention(witness_method: NonmutatingProtocol) <τ_0_0 where τ_0_0 : NonmutatingProtocol> (@in_guaranteed τ_0_0) -> Int
11961195
// CHECK-NEXT: destroy_addr [[C_FIELD_COPY]] : $*@opened("{{.*}}") NonmutatingProtocol
1197-
// CHECK-NEXT: assign [[RESULT_VALUE]] to [[UNINIT]] : $*Int
1196+
// CHECK-NEXT: assign [[RESULT_VALUE]] to [[UNINIT]]
1197+
// CHECK-NEXT: destroy_addr [[C_FIELD_BOX]]
11981198
// CHECK-NEXT: dealloc_stack [[C_FIELD_COPY]] : $*@opened("{{.*}}") NonmutatingProtocol
11991199
// CHECK-NEXT: dealloc_stack [[C_FIELD_BOX]] : $*NonmutatingProtocol
12001200
// CHECK-NEXT: dealloc_stack [[RESULT]] : $*Int

‎test/SILGen/protocols.swift

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,10 +26,10 @@ func use_subscript_rvalue_get(_ i : Int) -> Int {
2626
// CHECK: [[PROJ:%[0-9]+]] = open_existential_addr immutable_access [[READ]] : $*SubscriptableGet to $*[[OPENED:@opened(.*) SubscriptableGet]]
2727
// CHECK: [[ALLOCSTACK:%[0-9]+]] = alloc_stack $[[OPENED]]
2828
// CHECK: copy_addr [[PROJ]] to [initialization] [[ALLOCSTACK]] : $*[[OPENED]]
29-
// CHECK-NEXT: end_access [[READ]] : $*SubscriptableGet
3029
// CHECK-NEXT: [[METH:%[0-9]+]] = witness_method $[[OPENED]], #SubscriptableGet.subscript!getter.1
3130
// CHECK-NEXT: [[RESULT:%[0-9]+]] = apply [[METH]]<[[OPENED]]>(%0, [[ALLOCSTACK]])
3231
// CHECK-NEXT: destroy_addr [[ALLOCSTACK]]
32+
// CHECK-NEXT: end_access [[READ]] : $*SubscriptableGet
3333
// CHECK-NEXT: dealloc_stack [[ALLOCSTACK]] : $*[[OPENED]]
3434
// CHECK-NEXT: return [[RESULT]]
3535

@@ -133,9 +133,9 @@ func use_property_rvalue_get() -> Int {
133133
// CHECK: [[PROJ:%[0-9]+]] = open_existential_addr immutable_access [[READ]] : $*PropertyWithGetter to $*[[OPENED:@opened(.*) PropertyWithGetter]]
134134
// CHECK: [[COPY:%.*]] = alloc_stack $[[OPENED]]
135135
// CHECK-NEXT: copy_addr [[PROJ]] to [initialization] [[COPY]] : $*[[OPENED]]
136-
// CHECK-NEXT: end_access [[READ]] : $*PropertyWithGetter
137136
// CHECK-NEXT: [[METH:%[0-9]+]] = witness_method $[[OPENED]], #PropertyWithGetter.a!getter.1
138137
// CHECK-NEXT: apply [[METH]]<[[OPENED]]>([[COPY]])
138+
// CHECK: end_access [[READ]] : $*PropertyWithGetter
139139

140140
func use_property_lvalue_get() -> Int {
141141
return propertyGetSet.b
@@ -457,3 +457,15 @@ public func test(_ p: Proto) {
457457
// CHECK-LABEL: sil_witness_table hidden StructWithStoredClassProperty: PropertyWithGetter module protocols {
458458
// CHECK-NEXT: method #PropertyWithGetter.a!getter.1: {{.*}} : @_T09protocols29StructWithStoredClassPropertyVAA0fC6GetterA2aDP1aSivgTW
459459
// CHECK-NEXT: }
460+
461+
//
462+
// rdar://problem/37031037
463+
//
464+
465+
protocol MethodWithDefaultArgGenerator {}
466+
extension MethodWithDefaultArgGenerator {
467+
mutating func foo(_ x: Int = 0) {}
468+
}
469+
func invokeMethodWithDefaultArg(x: inout MethodWithDefaultArgGenerator) {
470+
x.foo()
471+
}

0 commit comments

Comments
 (0)