Skip to content

Commit 51f8cd1

Browse files
committed
gain isolation for static stored property initializer expressions
Some globals, like static stored properties, are lazily initialized. For situations where we do a direct access to that property, we will first call a function that tries to initialize the var if needed, before returning the address of the global to perform the direct access. In this specific case, we were not on the right actor when invoking that function. fixes rdar://83411416
1 parent 386940c commit 51f8cd1

File tree

5 files changed

+101
-10
lines changed

5 files changed

+101
-10
lines changed

lib/SILGen/SILGenFunction.h

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -704,7 +704,9 @@ class LLVM_LIBRARY_VISIBILITY SILGenFunction
704704
/// Generate an ObjC-compatible destructor (-dealloc).
705705
void emitObjCDestructor(SILDeclRef dtor);
706706

707-
ManagedValue emitGlobalVariableRef(SILLocation loc, VarDecl *var);
707+
/// Generate code to obtain the address of the given global variable.
708+
ManagedValue emitGlobalVariableRef(SILLocation loc, VarDecl *var,
709+
Optional<ActorIsolation> actorIso);
708710

709711
/// Generate a lazy global initializer.
710712
void emitLazyGlobalInitializer(PatternBindingDecl *binding,

lib/SILGen/SILGenGlobalVariable.cpp

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
//===----------------------------------------------------------------------===//
1212

1313
#include "SILGenFunction.h"
14+
#include "ExecutorBreadcrumb.h"
1415
#include "ManagedValue.h"
1516
#include "Scope.h"
1617
#include "swift/AST/ASTMangler.h"
@@ -66,7 +67,8 @@ SILGlobalVariable *SILGenModule::getSILGlobalVariable(VarDecl *gDecl,
6667
}
6768

6869
ManagedValue
69-
SILGenFunction::emitGlobalVariableRef(SILLocation loc, VarDecl *var) {
70+
SILGenFunction::emitGlobalVariableRef(SILLocation loc, VarDecl *var,
71+
Optional<ActorIsolation> actorIso) {
7072
assert(!VarLocs.count(var));
7173

7274
if (var->isLazilyInitializedGlobal()) {
@@ -75,7 +77,21 @@ SILGenFunction::emitGlobalVariableRef(SILLocation loc, VarDecl *var) {
7577
SILDeclRef(var, SILDeclRef::Kind::GlobalAccessor),
7678
NotForDefinition);
7779
SILValue accessor = B.createFunctionRefFor(loc, accessorFn);
80+
81+
// The accessor to obtain a global's address may need to initialize the
82+
// variable first. So, we must call this accessor with the same
83+
// isolation that the variable itself requires during access.
84+
ExecutorBreadcrumb prevExecutor = emitHopToTargetActor(loc, actorIso,
85+
/*base=*/None);
86+
7887
SILValue addr = B.createApply(loc, accessor, SubstitutionMap(), {});
88+
89+
// FIXME: often right after this, we will again hop to the actor to
90+
// read from this address. it would be better to merge these two hops
91+
// pairs of hops together. Alternatively, teaching optimizations to
92+
// expand the scope of two nearly-adjacent pairs would be good.
93+
prevExecutor.emit(*this, loc); // hop back after call.
94+
7995
// FIXME: It'd be nice if the result of the accessor was natively an
8096
// address.
8197
addr = B.createPointerToAddress(

lib/SILGen/SILGenLValue.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2813,7 +2813,7 @@ void LValue::addNonMemberVarComponent(SILGenFunction &SGF, SILLocation loc,
28132813

28142814
// The only other case that should get here is a global variable.
28152815
if (!address) {
2816-
address = SGF.emitGlobalVariableRef(Loc, Storage);
2816+
address = SGF.emitGlobalVariableRef(Loc, Storage, ActorIso);
28172817
} else {
28182818
assert(!ActorIso && "local var should not be actor isolated!");
28192819
}

test/Concurrency/Runtime/mainactor.swift

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,22 @@ actor A {
6868
return await enterMainActor(0) + 1
6969
}
7070

71+
@MainActor func mainActorFn() -> Int {
72+
checkIfMainQueue(expectedAnswer: true)
73+
return 10
74+
}
75+
76+
@MainActor
77+
struct S {
78+
static var bacteria: Int = mainActorFn()
79+
}
80+
81+
@MainActor
82+
class C {
83+
static var bacteria: Int = mainActorFn()
84+
lazy var amoeba: Int = mainActorFn()
85+
nonisolated init() {}
86+
}
7187

7288
// CHECK: starting
7389
// CHECK-NOT: ERROR
@@ -115,5 +131,15 @@ actor A {
115131
}
116132
}
117133
_ = await task.value
134+
135+
// Check that initializers for stored properties are on the right actor
136+
let t1 = Task.detached { () -> Int in
137+
let c = C()
138+
return await c.amoeba
139+
}
140+
let t2 = Task.detached { () -> Int in
141+
return await S.bacteria + C.bacteria
142+
}
143+
_ = await t1.value + t2.value
118144
}
119145
}

test/SILGen/hop_to_executor_async_prop.swift

Lines changed: 54 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -488,6 +488,10 @@ struct Container {
488488
}
489489

490490
// CHECK-LABEL: sil hidden [ossa] @$s4test9ContainerV12accessTuple3SfyYaF : $@convention(method) @async (@guaranteed Container) -> Float {
491+
// CHECK: [[ADDRESS_ACCESSOR:%[0-9]+]] = function_ref @$s4test9ContainerV12staticCircleSi_SitSg_Sftvau : $@convention(thin) () -> Builtin.RawPointer
492+
// CHECK: hop_to_executor {{%[0-9]+}} : $MainActor
493+
// CHECK: = apply [[ADDRESS_ACCESSOR]]() : $@convention(thin) () -> Builtin.RawPointer
494+
// CHECK: hop_to_executor {{%[0-9]+}} : $Optional<Builtin.Executor>
491495
// CHECK: hop_to_executor {{%[0-9]+}} : $MainActor
492496
// CHECK: [[ACCESS:%[0-9]+]] = begin_access [read] [dynamic] {{%[0-9]+}} : $*(Optional<(Int, Int)>, Float)
493497
// CHECK: [[ADDR:%[0-9]+]] = tuple_element_addr [[ACCESS]] : $*(Optional<(Int, Int)>, Float), 1
@@ -500,6 +504,10 @@ struct Container {
500504
}
501505

502506
// CHECK-LABEL: sil hidden [ossa] @$s4test9ContainerV12accessTuple4SiSgyYaFZ : $@convention(method) @async (@thin Container.Type) -> Optional<Int> {
507+
// CHECK: [[ADDRESS_ACCESSOR:%[0-9]+]] = function_ref @$s4test9ContainerV12staticCircleSi_SitSg_Sftvau : $@convention(thin) () -> Builtin.RawPointer
508+
// CHECK: hop_to_executor {{%[0-9]+}} : $MainActor
509+
// CHECK: = apply [[ADDRESS_ACCESSOR]]() : $@convention(thin) () -> Builtin.RawPointer
510+
// CHECK: hop_to_executor {{%[0-9]+}} : $Optional<Builtin.Executor>
503511
// CHECK: hop_to_executor {{%[0-9]+}} : $MainActor
504512
// CHECK: [[ACCESS:%[0-9]+]] = begin_access [read] [dynamic] {{%[0-9]+}} : $*(Optional<(Int, Int)>, Float)
505513
// CHECK: [[ADDR:%[0-9]+]] = tuple_element_addr [[ACCESS]] : $*(Optional<(Int, Int)>, Float), 0
@@ -522,6 +530,10 @@ struct Container {
522530

523531

524532
// CHECK-LABEL: sil hidden [ossa] @$s4test9ContainerV8getCountSiyYaFZ : $@convention(method) @async (@thin Container.Type) -> Int {
533+
// CHECK: [[ADDRESS_ACCESSOR:%[0-9]+]] = function_ref @$s4test9ContainerV7counterSivau : $@convention(thin) () -> Builtin.RawPointer
534+
// CHECK: hop_to_executor {{%[0-9]+}} : $MainActor
535+
// CHECK: = apply [[ADDRESS_ACCESSOR]]() : $@convention(thin) () -> Builtin.RawPointer
536+
// CHECK: hop_to_executor {{%[0-9]+}} : $Optional<Builtin.Executor>
525537
// CHECK: hop_to_executor {{%[0-9]+}} : $MainActor
526538
// CHECK: {{%[0-9]+}} = begin_access [read] [dynamic] {{%[0-9]+}} : $*Int
527539
// CHECK: {{%[0-9]+}} = load [trivial] {{%[0-9]+}} : $*Int
@@ -534,6 +546,10 @@ struct Container {
534546

535547
// CHECK-LABEL: sil hidden [ossa] @$s4test9ContainerV8getValueSiSgyYaFZ : $@convention(method) @async (@thin Container.Type) -> Optional<Int> {
536548
// CHECK: bb0(%0 : $@thin Container.Type):
549+
// CHECK: [[ADDRESS_ACCESSOR:%[0-9]+]] = function_ref @$s4test9ContainerV4thisACSgvau : $@convention(thin) () -> Builtin.RawPointer
550+
// CHECK: hop_to_executor {{%[0-9]+}} : $MainActor
551+
// CHECK: = apply [[ADDRESS_ACCESSOR]]() : $@convention(thin) () -> Builtin.RawPointer
552+
// CHECK: hop_to_executor {{%[0-9]+}} : $Optional<Builtin.Executor>
537553
// CHECK: [[MAIN:%[0-9]+]] = begin_borrow {{%[0-9]+}} : $MainActor
538554
// CHECK: [[PREV:%[0-9]+]] = builtin "getCurrentExecutor"() : $Optional<Builtin.Executor>
539555
// CHECK: hop_to_executor [[MAIN]] : $MainActor
@@ -556,19 +572,23 @@ struct Container {
556572

557573
// CHECK-LABEL: sil hidden [ossa] @$s4test9ContainerV10getOrCrashSfyYaFZ : $@convention(method) @async (@thin Container.Type) -> Float {
558574
// CHECK: bb0({{%[0-9]+}} : $@thin Container.Type):
575+
// CHECK: [[ADDRESS_ACCESSOR:%[0-9]+]] = function_ref @$s4test9ContainerV4thisACSgvau : $@convention(thin) () -> Builtin.RawPointer
576+
// CHECK: hop_to_executor {{%[0-9]+}} : $MainActor
577+
// CHECK: = apply [[ADDRESS_ACCESSOR]]() : $@convention(thin) () -> Builtin.RawPointer
578+
// CHECK: hop_to_executor {{%[0-9]+}} : $Optional<Builtin.Executor>
559579
// CHECK: [[MAIN:%[0-9]+]] = begin_borrow {{%[0-9]+}} : $MainActor
560580
// CHECK: [[PREV:%[0-9]+]] = builtin "getCurrentExecutor"() : $Optional<Builtin.Executor>
561581
// CHECK: hop_to_executor [[MAIN]] : $MainActor
562582
// CHECK: [[ACCESS:%[0-9]+]] = begin_access [read] [dynamic] {{%[0-9]+}} : $*Optional<Container>
563-
// CHECK: switch_enum_addr %11 : $*Optional<Container>, case #Optional.some!enumelt: [[SOME_BB:bb[0-9]+]], case #Optional.none!enumelt: [[CRASH_BB:bb[0-9]+]]
583+
// CHECK: switch_enum_addr [[ACCESS]] : $*Optional<Container>, case #Optional.some!enumelt: [[SOME_BB:bb[0-9]+]], case #Optional.none!enumelt: [[CRASH_BB:bb[0-9]+]]
564584
//
565585
// CHECK: [[CRASH_BB]]:
566586
// CHECK-NOT: hop_to_executor {{%[0-9]+}}
567587
// CHECK: unreachable
568588
//
569589
// CHECK: [[SOME_BB]]:
570-
// CHECK: [[DATA_ADDR:%[0-9]+]] = unchecked_take_enum_data_addr %11 : $*Optional<Container>, #Optional.some!enumelt
571-
// CHECK: [[ELEM_ADDR:%[0-9]+]] = struct_element_addr %22 : $*Container, #Container.iso
590+
// CHECK: [[DATA_ADDR:%[0-9]+]] = unchecked_take_enum_data_addr [[ACCESS]] : $*Optional<Container>, #Optional.some!enumelt
591+
// CHECK: [[ELEM_ADDR:%[0-9]+]] = struct_element_addr [[DATA_ADDR]] : $*Container, #Container.iso
572592
// CHECK: [[PREV_AGAIN:%[0-9]+]] = builtin "getCurrentExecutor"() : $Optional<Builtin.Executor>
573593
// CHECK: hop_to_executor {{%[0-9]+}} : $Cat
574594
// CHECK: {{%[0-9]+}} = load [trivial] [[ELEM_ADDR]] : $*Float
@@ -581,19 +601,22 @@ struct Container {
581601

582602
// CHECK-LABEL: sil hidden [ossa] @$s4test9ContainerV13getRefOrCrashAA6CatBoxCyYaFZ : $@convention(method) @async (@thin Container.Type) -> @owned CatBox {
583603
// CHECK: bb0({{%[0-9]+}} : $@thin Container.Type):
604+
// CHECK: [[ADDRESS_ACCESSOR:%[0-9]+]] = function_ref @$s4test9ContainerV4thisACSgvau : $@convention(thin) () -> Builtin.RawPointer
605+
// CHECK: hop_to_executor {{%[0-9]+}} : $MainActor
606+
// CHECK: = apply [[ADDRESS_ACCESSOR]]() : $@convention(thin) () -> Builtin.RawPointer
607+
// CHECK: hop_to_executor {{%[0-9]+}} : $Optional<Builtin.Executor>
584608
// CHECK: [[MAIN:%[0-9]+]] = begin_borrow {{%[0-9]+}} : $MainActor
585609
// CHECK: [[PREV:%[0-9]+]] = builtin "getCurrentExecutor"() : $Optional<Builtin.Executor>
586610
// CHECK: hop_to_executor [[MAIN]] : $MainActor
587611
// CHECK: [[ACCESS:%[0-9]+]] = begin_access [read] [dynamic] {{%[0-9]+}} : $*Optional<Container>
588-
// CHECK: switch_enum_addr %11 : $*Optional<Container>, case #Optional.some!enumelt: [[SOME_BB:bb[0-9]+]], case #Optional.none!enumelt: [[CRASH_BB:bb[0-9]+]]
612+
// CHECK: switch_enum_addr [[ACCESS]] : $*Optional<Container>, case #Optional.some!enumelt: [[SOME_BB:bb[0-9]+]], case #Optional.none!enumelt: [[CRASH_BB:bb[0-9]+]]
589613
//
590614
// CHECK: [[CRASH_BB]]:
591-
// CHECK-NOT: hop_to_executor {{%[0-9]+}}
592615
// CHECK: unreachable
593616
//
594617
// CHECK: [[SOME_BB]]:
595-
// CHECK: [[DATA_ADDR:%[0-9]+]] = unchecked_take_enum_data_addr %11 : $*Optional<Container>, #Optional.some!enumelt
596-
// CHECK: [[ELEM_ADDR:%[0-9]+]] = struct_element_addr %22 : $*Container, #Container.iso
618+
// CHECK: [[DATA_ADDR:%[0-9]+]] = unchecked_take_enum_data_addr [[ACCESS]] : $*Optional<Container>, #Optional.some!enumelt
619+
// CHECK: [[ELEM_ADDR:%[0-9]+]] = struct_element_addr [[DATA_ADDR]] : $*Container, #Container.iso
597620
// CHECK: [[PREV_AGAIN:%[0-9]+]] = builtin "getCurrentExecutor"() : $Optional<Builtin.Executor>
598621
// CHECK: hop_to_executor {{%[0-9]+}} : $Cat
599622
// CHECK: {{%[0-9]+}} = load [copy] [[ELEM_ADDR]] : $*CatBox
@@ -644,3 +667,27 @@ struct Blah {
644667
}
645668
}
646669
}
670+
671+
@MainActor
672+
func getTemperature() -> Int { return 0 }
673+
674+
@MainActor
675+
class Polar {
676+
static var temperature: Int = getTemperature()
677+
}
678+
679+
680+
// CHECK-LABEL: sil hidden{{.*}} @$s4test20accessStaticIsolatedSiyYaF : $@convention(thin) @async () -> Int {
681+
// CHECK: [[ADDRESSOR:%[0-9]+]] = function_ref @$s4test5PolarC11temperatureSivau : $@convention(thin) () -> Builtin.RawPointer
682+
// CHECK: hop_to_executor {{%.*}} : $MainActor
683+
// CHECK-NEXT: [[RAW_ADDR:%[0-9]+]] = apply [[ADDRESSOR]]() : $@convention(thin) () -> Builtin.RawPointer
684+
// CHECK-NEXT: hop_to_executor {{%.*}} : $Optional<Builtin.Executor>
685+
// CHECK: [[ADDR:%[0-9]+]] = pointer_to_address [[RAW_ADDR]] : $Builtin.RawPointer to [strict] $*Int
686+
// CHECK: hop_to_executor {{%.*}} : $MainActor
687+
// CHECK: {{%.*}} = load [trivial] {{%.*}} : $*Int
688+
// CHECK: hop_to_executor {{%.*}} : $Optional<Builtin.Executor>
689+
func accessStaticIsolated() async -> Int {
690+
return await Polar.temperature
691+
}
692+
693+

0 commit comments

Comments
 (0)