Skip to content

Commit 3c7ad60

Browse files
committed
[ShrinkBorrowScope] Adopt VisitBarrierAccessScopes.
Avoids hoisting borrow scopes into unrelated access scopes which could introduce exclusivity violations. rdar://93060369
1 parent 180095a commit 3c7ad60

File tree

2 files changed

+129
-1
lines changed

2 files changed

+129
-1
lines changed

lib/SILOptimizer/Utils/ShrinkBorrowScope.cpp

Lines changed: 56 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
#include "swift/SIL/SILBasicBlock.h"
2222
#include "swift/SIL/SILInstruction.h"
2323
#include "swift/SILOptimizer/Analysis/Reachability.h"
24+
#include "swift/SILOptimizer/Analysis/VisitBarrierAccessScopes.h"
2425
#include "swift/SILOptimizer/Utils/CanonicalizeBorrowScope.h"
2526
#include "swift/SILOptimizer/Utils/InstOptUtils.h"
2627
#include "swift/SILOptimizer/Utils/InstructionDeleter.h"
@@ -134,11 +135,14 @@ struct DeinitBarriers final {
134135
DeinitBarriers &operator=(DeinitBarriers const &) = delete;
135136
};
136137

138+
class BarrierAccessScopeFinder;
139+
137140
/// Works backwards from the current location of end_borrows to the earliest
138141
/// place they can be hoisted to.
139142
///
140143
/// Implements IterativeBackwardReachability::Effects.
141144
/// Implements IterativeBackwardReachability::findBarrier::Visitor.
145+
/// Implements VisitBarrierAccessScopes::Effects
142146
class Dataflow final {
143147
public:
144148
using Reachability = IterativeBackwardReachability<Dataflow>;
@@ -167,19 +171,30 @@ class Dataflow final {
167171

168172
private:
169173
friend Reachability;
174+
friend class BarrierAccessScopeFinder;
175+
friend class VisitBarrierAccessScopes<Dataflow, BarrierAccessScopeFinder>;
170176

171177
Classification classifyInstruction(SILInstruction *);
172178

173179
bool classificationIsBarrier(Classification);
174180

175-
/// Implements IterativeBackwardReachability::Effects.
181+
/// IterativeBackwardReachability::Effects
182+
/// VisitBarrierAccessScopes::Effects
176183

177184
ArrayRef<SILInstruction *> gens() { return uses.ends; }
178185

179186
Effect effectForInstruction(SILInstruction *);
180187

181188
Effect effectForPhi(SILBasicBlock *);
182189

190+
/// VisitBarrierAccessScopes::Effects
191+
192+
auto localGens() { return result.localGens; }
193+
194+
bool isLocalGen(SILInstruction *instruction) {
195+
return result.localGens.contains(instruction);
196+
}
197+
183198
/// IterativeBackwardReachability::findBarrier::Visitor.
184199

185200
void visitBarrierInstruction(SILInstruction *instruction) {
@@ -224,6 +239,11 @@ Dataflow::classifyInstruction(SILInstruction *instruction) {
224239
if (uses.users.contains(instruction)) {
225240
return Classification::Barrier;
226241
}
242+
if (auto *eai = dyn_cast<EndAccessInst>(instruction)) {
243+
return barrierAccessScopes.contains(eai->getBeginAccess())
244+
? Classification::Barrier
245+
: Classification::Other;
246+
}
227247
if (isDeinitBarrier(instruction)) {
228248
return Classification::Barrier;
229249
}
@@ -263,8 +283,43 @@ Dataflow::Effect Dataflow::effectForPhi(SILBasicBlock *block) {
263283
return isBarrier ? Effect::Kill() : Effect::NoEffect();
264284
}
265285

286+
/// Finds end_access instructions which are barriers to hoisting because the
287+
/// access scopes they contain barriers to hoisting. Hoisting end_borrows into
288+
/// such access scopes could introduce exclusivity violations.
289+
///
290+
/// Implements BarrierAccessScopeFinder::Visitor
291+
class BarrierAccessScopeFinder final {
292+
using Impl = VisitBarrierAccessScopes<Dataflow, BarrierAccessScopeFinder>;
293+
Context const &context;
294+
Impl impl;
295+
Dataflow &dataflow;
296+
297+
public:
298+
BarrierAccessScopeFinder(Context const &context, Dataflow &dataflow)
299+
: context(context), impl(&context.function, dataflow, *this),
300+
dataflow(dataflow) {}
301+
302+
void find() { impl.visit(); }
303+
304+
private:
305+
friend Impl;
306+
307+
bool isInRegion(SILBasicBlock *block) {
308+
return dataflow.result.discoveredBlocks.contains(block);
309+
}
310+
311+
void visitBarrierAccessScope(BeginAccessInst *bai) {
312+
dataflow.barrierAccessScopes.insert(bai);
313+
for (auto *eai : bai->getEndAccesses()) {
314+
dataflow.reachability.addKill(eai);
315+
}
316+
}
317+
};
318+
266319
void Dataflow::run() {
267320
reachability.initialize();
321+
BarrierAccessScopeFinder finder(context, *this);
322+
finder.find();
268323
reachability.solve();
269324
recordCopies = true;
270325
reachability.findBarriers(*this);

test/SILOptimizer/shrink_borrow_scope.sil

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1097,3 +1097,76 @@ bb0(%0 : $*ClassWrapper):
10971097
// =============================================================================
10981098
// instruction tests }}
10991099
// =============================================================================
1100+
1101+
// =============================================================================
1102+
// access scope tests {{
1103+
// =============================================================================
1104+
1105+
// Don't hoist into an access scope that contains a barrier.
1106+
//
1107+
// CHECK-LABEL: sil [ossa] @nofold_scoped_load_barrier : {{.*}} {
1108+
// CHECK: end_access
1109+
// CHECK: end_access
1110+
// CHECK: end_borrow
1111+
// CHECK-LABEL: // end sil function 'nofold_scoped_load_barrier'
1112+
sil [ossa] @nofold_scoped_load_barrier : $@convention(thin) (@owned C, @owned C) -> (@owned C) {
1113+
entry(%instance : @owned $C, %other : @owned $C):
1114+
%lifetime = begin_borrow [lexical] %instance : $C
1115+
apply undef(%lifetime) : $@convention(thin) (@guaranteed C) -> ()
1116+
%addr = alloc_stack $C
1117+
%store_scope = begin_access [modify] [static] %addr : $*C
1118+
store %other to [init] %store_scope : $*C
1119+
end_access %store_scope : $*C
1120+
%load_scope = begin_access [read] [static] %addr : $*C
1121+
%value = load [copy] %load_scope : $*C
1122+
%barrier = function_ref @barrier : $@convention(thin) () -> ()
1123+
apply %barrier() : $@convention(thin) () -> ()
1124+
end_access %load_scope : $*C
1125+
destroy_addr %addr : $*C
1126+
dealloc_stack %addr : $*C
1127+
end_borrow %lifetime : $C
1128+
destroy_value %instance : $C
1129+
return %value : $C
1130+
}
1131+
1132+
// Access scopes that are open at barrier blocks are barriers. Otherwise, we
1133+
// would hoist end_borrows into the scopes when the end_borrows are hoisted up
1134+
// to the begin of blocks whose predecessor is the barrier block.
1135+
//
1136+
// CHECK-LABEL: sil [ossa] @nohoist_into_access_scope_barred_by_barrier_block : {{.*}} {
1137+
// CHECK: {{bb[0-9]+}}([[INSTANCE:%[^,]+]] : @owned $C, [[INOUT:%[^,]+]] : $*C):
1138+
// CHECK: [[LIFETIME:%[^,]+]] = begin_borrow [lexical] [[INSTANCE]]
1139+
// CHECK: [[SCOPE:%[^,]+]] = begin_access [modify] [static] [[INOUT]] : $*C
1140+
// CHECK: cond_br undef, [[LEFT:bb[0-9]+]],
1141+
// CHECK: [[LEFT]]:
1142+
// CHECK: end_access [[SCOPE]] : $*C
1143+
// CHECK-NEXT: end_borrow [[LIFETIME]] : $C
1144+
// CHECK-LABEL: } // end sil function 'nohoist_into_access_scope_barred_by_barrier_block'
1145+
sil [ossa] @nohoist_into_access_scope_barred_by_barrier_block : $@convention(thin) (@owned C, @inout C) -> () {
1146+
entry(%instance : @owned $C, %second : $*C):
1147+
%lifetime = begin_borrow [lexical] %instance : $C
1148+
%scope = begin_access [modify] [static] %second : $*C
1149+
cond_br undef, left, right
1150+
1151+
left:
1152+
end_access %scope : $*C
1153+
%ignore = tuple ()
1154+
end_borrow %lifetime : $C
1155+
destroy_value %instance : $C
1156+
br exit
1157+
1158+
right:
1159+
end_access %scope : $*C
1160+
apply undef(%lifetime) : $@convention(thin) (@guaranteed C) -> ()
1161+
end_borrow %lifetime : $C
1162+
destroy_value %instance : $C
1163+
br exit
1164+
1165+
exit:
1166+
%retval = tuple ()
1167+
return %retval : $()
1168+
}
1169+
1170+
// =============================================================================
1171+
// access scope tests }}
1172+
// =============================================================================

0 commit comments

Comments
 (0)