Skip to content

Commit 53028ab

Browse files
aidan-hallaidan-hall
authored
Merge pull request swiftlang#83665 from aidan-hall/fix-effects
ComputeSideEffects: compute properties even if function has an effect attribute
2 parents 6f1b831 + 6a263f8 commit 53028ab

File tree

3 files changed

+231
-16
lines changed

3 files changed

+231
-16
lines changed

‎SwiftCompilerSources/Sources/Optimizer/FunctionPasses/ComputeSideEffects.swift‎

Lines changed: 82 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -37,43 +37,109 @@ let computeSideEffects = FunctionPass(name: "compute-side-effects") {
3737
return
3838
}
3939

40-
if function.effectAttribute != .none {
41-
// Don't try to infer side effects if there are defined effect attributes.
42-
return
43-
}
44-
4540
var collectedEffects = CollectedEffects(function: function, context)
4641

47-
// First step: collect effects from all instructions.
48-
//
42+
// Collect effects from all instructions.
4943
for block in function.blocks {
5044
for inst in block.instructions {
5145
collectedEffects.addInstructionEffects(inst)
5246
}
5347
}
54-
55-
// Second step: If an argument has unknown uses, we must add all previously collected
56-
// global effects to the argument, because we don't know to which "global" side-effect
57-
// instruction the argument might have escaped.
48+
// If an argument has unknown uses, we must add all previously collected
49+
// global effects to the argument, because we don't know to which "global"
50+
// side-effect instruction the argument might have escaped.
5851
for argument in function.arguments {
5952
collectedEffects.addEffectsForEscapingArgument(argument: argument)
6053
collectedEffects.addEffectsForConsumingArgument(argument: argument)
6154
}
6255

56+
let globalEffects: SideEffects.GlobalEffects
57+
do {
58+
let computed = collectedEffects.globalEffects
59+
60+
// Combine computed global effects with effects defined by the function's effect attribute, if it has one.
61+
62+
// The defined and computed global effects of a function with an effect attribute should be treated as
63+
// worst case global effects of the function.
64+
// This means a global effect should only occur iff it is computed AND defined to occur.
65+
66+
let defined = function.definedGlobalEffects
67+
68+
globalEffects = SideEffects.GlobalEffects(
69+
memory: SideEffects.Memory(read: defined.memory.read && computed.memory.read,
70+
write: defined.memory.write && computed.memory.write),
71+
ownership: SideEffects.Ownership(copy: defined.ownership.copy && computed.ownership.copy,
72+
destroy: defined.ownership.destroy && computed.ownership.destroy),
73+
allocates: defined.allocates && computed.allocates,
74+
isDeinitBarrier: defined.isDeinitBarrier && computed.isDeinitBarrier
75+
)
76+
}
77+
78+
// Obtain the argument effects of the function.
79+
var argumentEffects = collectedEffects.argumentEffects
80+
81+
// `[readnone]` and `[readonly]` functions can still access the value fields
82+
// of their indirect arguments, permitting v** read and write effects. If
83+
// additional read or write effects are computed, we can replace.
84+
switch function.effectAttribute {
85+
case .readNone:
86+
for i in (0..<argumentEffects.count) {
87+
// Even a `[readnone]` function can read from indirect arguments.
88+
if !function.argumentConventions[i].isIndirectIn {
89+
argumentEffects[i].read = nil
90+
} else if argumentEffects[i].read?.mayHaveClassProjection ?? false {
91+
argumentEffects[i].read = SmallProjectionPath(.anyValueFields)
92+
}
93+
94+
// Even a `[readnone]` function can write to indirect results.
95+
if !function.argument(at: i).isIndirectResult {
96+
argumentEffects[i].write = nil
97+
} else if argumentEffects[i].write?.mayHaveClassProjection ?? false {
98+
argumentEffects[i].write = SmallProjectionPath(.anyValueFields)
99+
}
100+
101+
argumentEffects[i].copy = nil
102+
argumentEffects[i].destroy = nil
103+
}
104+
105+
case .readOnly:
106+
for i in (0..<argumentEffects.count) {
107+
// Even a `[readonly]` function can write to indirect results.
108+
if !function.argument(at: i).isIndirectResult {
109+
argumentEffects[i].write = nil
110+
} else if argumentEffects[i].write?.mayHaveClassProjection ?? false {
111+
argumentEffects[i].write = SmallProjectionPath(.anyValueFields)
112+
}
113+
114+
argumentEffects[i].destroy = nil
115+
}
116+
117+
case .releaseNone:
118+
for i in (0..<argumentEffects.count) {
119+
// A `[releasenone]` function can do anything except destroy an argument.
120+
argumentEffects[i].destroy = nil
121+
}
122+
123+
case .none:
124+
// The user makes no additional guarantees about the effects of the function.
125+
break
126+
}
127+
128+
63129
// Don't modify the effects if they didn't change. This avoids sending a change notification
64130
// which can trigger unnecessary other invalidations.
65131
if let existingEffects = function.effects.sideEffects,
66-
existingEffects.arguments == collectedEffects.argumentEffects,
67-
existingEffects.global == collectedEffects.globalEffects {
132+
existingEffects.arguments == argumentEffects,
133+
existingEffects.global == globalEffects {
68134
return
69135
}
70136

71137
// Finally replace the function's side effects.
72138
function.modifyEffects(context) { (effects: inout FunctionEffects) in
73139
let globalEffects = function.isProgramTerminationPoint ?
74-
collectedEffects.globalEffects.forProgramTerminationPoints
75-
: collectedEffects.globalEffects
76-
effects.sideEffects = SideEffects(arguments: collectedEffects.argumentEffects, global: globalEffects)
140+
globalEffects.forProgramTerminationPoints
141+
: globalEffects
142+
effects.sideEffects = SideEffects(arguments: argumentEffects, global: globalEffects)
77143
}
78144
}
79145

‎test/SILOptimizer/side_effects.sil‎

Lines changed: 116 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1281,3 +1281,119 @@ bb0(%0: $any Error):
12811281
%r = tuple ()
12821282
return %r : $()
12831283
}
1284+
1285+
// Base case: @destroy_not_escaped_closure_test. It has every global and argument effect, so we can see which are removed.
1286+
// CHECK-LABEL: sil [readnone] @test_effect_attribute_readnone
1287+
// CHECK-NEXT: [global: copy,deinit_barrier]
1288+
// CHECK-NEXT: {{^[^[]}}
1289+
// CHECK-LABEL: } // end sil function 'test_effect_attribute_readnone'
1290+
sil [readnone] @test_effect_attribute_readnone : $@convention(thin)(@owned @callee_guaranteed () -> ()) -> Builtin.Int1 {
1291+
bb0(%0 : $@callee_guaranteed () -> ()):
1292+
%1 = destroy_not_escaped_closure %0: $@callee_guaranteed () -> ()
1293+
return %1 : $Builtin.Int1
1294+
}
1295+
1296+
// CHECK-LABEL: sil [readonly] @test_effect_attribute_readonly
1297+
// CHECK-NEXT: [%0: read v**.c*.v**, copy v**.c*.v**]
1298+
// CHECK-NEXT: [global: read,copy,deinit_barrier]
1299+
// CHECK-NEXT: {{^[^[]}}
1300+
// CHECK-LABEL: } // end sil function 'test_effect_attribute_readonly'
1301+
sil [readonly] @test_effect_attribute_readonly : $@convention(thin)(@owned @callee_guaranteed () -> ()) -> Builtin.Int1 {
1302+
bb0(%0 : $@callee_guaranteed () -> ()):
1303+
%1 = destroy_not_escaped_closure %0: $@callee_guaranteed () -> ()
1304+
return %1 : $Builtin.Int1
1305+
}
1306+
1307+
// CHECK-LABEL: sil [releasenone] @test_effect_attribute_releasenone
1308+
// CHECK-NEXT: [%0: read v**.c*.v**, write v**.c*.v**, copy v**.c*.v**]
1309+
// CHECK-NEXT: [global: read,write,copy,allocate,deinit_barrier]
1310+
// CHECK-NEXT: {{^[^[]}}
1311+
// CHECK-LABEL: } // end sil function 'test_effect_attribute_releasenone'
1312+
sil [releasenone] @test_effect_attribute_releasenone : $@convention(thin)(@owned @callee_guaranteed () -> ()) -> Builtin.Int1 {
1313+
bb0(%0 : $@callee_guaranteed () -> ()):
1314+
%1 = destroy_not_escaped_closure %0: $@callee_guaranteed () -> ()
1315+
return %1 : $Builtin.Int1
1316+
}
1317+
1318+
// These tests are adapted from the test case in rdar://155870190
1319+
sil @test_effect_attribute_no_barrier : $@convention(thin) () -> () {
1320+
[global: ]
1321+
}
1322+
1323+
sil @test_effect_attribute_barrier : $@convention(thin) () -> () {
1324+
[global: deinit_barrier]
1325+
}
1326+
// CHECK-LABEL: sil [readnone] @test_effect_attribute_call_no_barrier
1327+
// CHECK-NEXT: [global: ]
1328+
// CHECK-NEXT: {{^[^[]}}
1329+
// CHECK-LABEL: } // end sil function 'test_effect_attribute_call_no_barrier'
1330+
sil [readnone] @test_effect_attribute_call_no_barrier : $@convention(thin) () -> @out Any {
1331+
bb0(%0 : $*Any):
1332+
%2 = function_ref @test_effect_attribute_no_barrier : $@convention(thin) () -> ()
1333+
apply %2() : $@convention(thin) () -> ()
1334+
%4 = tuple ()
1335+
return %4
1336+
}
1337+
1338+
// CHECK-LABEL: sil [readnone] @test_effect_attribute_call_barrier
1339+
// CHECK-NEXT: [global: deinit_barrier]
1340+
// CHECK-NEXT: {{^[^[]}}
1341+
// CHECK-LABEL: } // end sil function 'test_effect_attribute_call_barrier'
1342+
sil [readnone] @test_effect_attribute_call_barrier : $@convention(thin) () -> @out Any {
1343+
bb0(%0 : $*Any):
1344+
%2 = function_ref @test_effect_attribute_barrier : $@convention(thin) () -> ()
1345+
apply %2() : $@convention(thin) () -> ()
1346+
%4 = tuple ()
1347+
return %4
1348+
}
1349+
1350+
// The readnone attribute can not rule out reads from indirect arguments.
1351+
// CHECK-LABEL: sil [readnone] @test_effect_attribute_readnone_direct_argument
1352+
// CHECK-NEXT: [global: ]
1353+
// CHECK-NEXT: {{^[^[]}}
1354+
// CHECK-LABEL: } // end sil function 'test_effect_attribute_readnone_direct_argument'
1355+
sil [readnone] @test_effect_attribute_readnone_direct_argument : $@convention(thin) (Int) -> Int {
1356+
bb0(%0 : $Int):
1357+
return %0
1358+
}
1359+
1360+
// CHECK-LABEL: sil [readnone] @test_effect_attribute_readnone_indirect_argument
1361+
// CHECK-NEXT: [%0: read v**]
1362+
// CHECK-NEXT: [global: ]
1363+
// CHECK-NEXT: {{^[^[]}}
1364+
// CHECK-LABEL: } // end sil function 'test_effect_attribute_readnone_indirect_argument'
1365+
sil [readnone] @test_effect_attribute_readnone_indirect_argument : $@convention(thin) (@in Int) -> Int {
1366+
bb0(%0 : $*Int):
1367+
%1 = load %0
1368+
return %1
1369+
}
1370+
1371+
// Base case: @retain_and_store. The indirect result has a write effect, which readnone & readonly can't rule out.
1372+
// CHECK-LABEL: sil [readnone] @test_effect_attribute_readnone_indirect_result
1373+
// CHECK-NEXT: [%0: write v**]
1374+
// CHECK-NEXT: [global: ]
1375+
// CHECK-NEXT: {{^[^[]}}
1376+
// CHECK-LABEL: } // end sil function 'test_effect_attribute_readnone_indirect_result'
1377+
sil [readnone] @test_effect_attribute_readnone_indirect_result : $@convention(thin) (@guaranteed X) -> @out X {
1378+
bb0(%0 : $*X, %1 : $X):
1379+
strong_retain %1 : $X
1380+
store %1 to %0 : $*X
1381+
1382+
%r = tuple ()
1383+
return %r : $()
1384+
}
1385+
1386+
// CHECK-LABEL: sil [readonly] @test_effect_attribute_readonly_indirect_result
1387+
// CHECK-NEXT: [%0: write v**]
1388+
// CHECK-NEXT: [%1: copy v**]
1389+
// CHECK-NEXT: [global: ]
1390+
// CHECK-NEXT: {{^[^[]}}
1391+
// CHECK-LABEL: } // end sil function 'test_effect_attribute_readonly_indirect_result'
1392+
sil [readonly] @test_effect_attribute_readonly_indirect_result : $@convention(thin) (@guaranteed X) -> @out X {
1393+
bb0(%0 : $*X, %1 : $X):
1394+
strong_retain %1 : $X
1395+
store %1 to %0 : $*X
1396+
1397+
%r = tuple ()
1398+
return %r : $()
1399+
}

‎test/SILOptimizer/templvalueopt_ossa.sil‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -336,6 +336,39 @@ bb0(%0 : @guaranteed $Any, %1 : $*Any):
336336
return %11 : $()
337337
}
338338

339+
sil [readnone] @createAny_effects : $@convention(thin) () -> @out Any
340+
sil [readnone] @createAny_no_barrier_effects : $@convention(thin) () -> @out Any {
341+
[global:]
342+
}
343+
344+
// CHECK-LABEL: sil [ossa] @test_deinit_barrier_effects :
345+
// CHECK: copy_addr [take]
346+
// CHECK-LABEL: } // end sil function 'test_deinit_barrier_effects'
347+
sil [ossa] @test_deinit_barrier_effects : $@convention(thin) (@guaranteed Any, @inout Any) -> () {
348+
bb0(%0 : @guaranteed $Any, %1 : $*Any):
349+
%2 = alloc_stack $Any
350+
%4 = function_ref @createAny_effects : $@convention(thin) () -> @out Any
351+
%5 = apply %4(%2) : $@convention(thin) () -> @out Any
352+
copy_addr [take] %2 to %1 : $*Any
353+
dealloc_stack %2 : $*Any
354+
%11 = tuple ()
355+
return %11 : $()
356+
}
357+
358+
// CHECK-LABEL: sil [ossa] @test_no_deinit_barrier_effects :
359+
// CHECK-NOT: copy_addr
360+
// CHECK-LABEL: } // end sil function 'test_no_deinit_barrier_effects'
361+
sil [ossa] @test_no_deinit_barrier_effects : $@convention(thin) (@guaranteed Any, @inout Any) -> () {
362+
bb0(%0 : @guaranteed $Any, %1 : $*Any):
363+
%2 = alloc_stack $Any
364+
%4 = function_ref @createAny_no_barrier_effects : $@convention(thin) () -> @out Any
365+
%5 = apply %4(%2) : $@convention(thin) () -> @out Any
366+
copy_addr [take] %2 to %1 : $*Any
367+
dealloc_stack %2 : $*Any
368+
%11 = tuple ()
369+
return %11 : $()
370+
}
371+
339372
struct TwoFields {
340373
var a: Child
341374
var b: Child

0 commit comments

Comments
 (0)