Skip to content

Commit fba6dfd

Browse files
DougGregorDougGregor
authored
Merge pull request #40692 from kavon/5.6-statics-isolation-fix
[5.6] fix implementation of isolation for static stored property initializer expressions
2 parents 0bf73bb + 6356226 commit fba6dfd

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)