Skip to content

Commit 9c17447

Browse files
Merge pull request #61654 from nate-chandler/shrink_borrow_scope/handle_barrier_merge_terminators
Removed deinit barrier workaround.
2 parents dc8d37d + f98917b commit 9c17447

File tree

6 files changed

+71
-47
lines changed

6 files changed

+71
-47
lines changed

SwiftCompilerSources/Sources/SIL/Instruction.swift

Lines changed: 3 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -112,35 +112,16 @@ public class Instruction : ListNode, CustomStringConvertible, Hashable {
112112
return swift_mayAccessPointer(bridged)
113113
}
114114

115-
/// Whether this instruction loads or copies a value whose storage does not
116-
/// increment the stored value's reference count.
117115
public final var mayLoadWeakOrUnowned: Bool {
118-
switch self {
119-
case is LoadWeakInst, is LoadUnownedInst, is StrongCopyUnownedValueInst, is StrongCopyUnmanagedValueInst:
120-
return true
121-
default:
122-
return false
123-
}
116+
return swift_mayLoadWeakOrUnowned(bridged)
124117
}
125118

126-
/// Conservatively, whether this instruction could involve a synchronization
127-
/// point like a memory barrier, lock or syscall.
128119
public final var maySynchronizeNotConsideringSideEffects: Bool {
129-
switch self {
130-
case is FullApplySite, is EndApplyInst, is AbortApplyInst:
131-
return true
132-
default:
133-
return false
134-
}
120+
return swift_maySynchronizeNotConsideringSideEffects(bridged)
135121
}
136122

137-
/// Conservatively, whether this instruction could be a barrier to hoisting
138-
/// destroys.
139-
///
140-
/// Does not consider function so effects, so every apply is treated as a
141-
/// barrier.
142123
public final var mayBeDeinitBarrierNotConsideringSideEffects: Bool {
143-
return mayAccessPointer || mayLoadWeakOrUnowned || maySynchronizeNotConsideringSideEffects
124+
return swift_mayBeDeinitBarrierNotConsideringSideEffects(bridged)
144125
}
145126

146127
public func visitReferencedFunctions(_ cl: (Function) -> ()) {

include/swift/SIL/MemAccessUtils.h

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -236,6 +236,21 @@ inline bool accessKindMayConflict(SILAccessKind a, SILAccessKind b) {
236236
/// such as by reading a pointer.
237237
bool mayAccessPointer(SILInstruction *instruction);
238238

239+
/// Whether this instruction loads or copies a value whose storage does not
240+
/// increment the stored value's reference count.
241+
bool mayLoadWeakOrUnowned(SILInstruction* instruction);
242+
243+
/// Conservatively, whether this instruction could involve a synchronization
244+
/// point like a memory barrier, lock or syscall.
245+
bool maySynchronizeNotConsideringSideEffects(SILInstruction* instruction);
246+
247+
/// Conservatively, whether this instruction could be a barrier to hoisting
248+
/// destroys.
249+
///
250+
/// Does not consider function so effects, so every apply is treated as a
251+
/// barrier.
252+
bool mayBeDeinitBarrierNotConsideringSideEffects(SILInstruction *instruction);
253+
239254
} // end namespace swift
240255

241256
//===----------------------------------------------------------------------===//

include/swift/SIL/SILBridging.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -349,6 +349,9 @@ BridgedMemoryBehavior SILInstruction_getMemBehavior(BridgedInstruction inst);
349349
bool SILInstruction_mayRelease(BridgedInstruction inst);
350350
bool SILInstruction_hasUnspecifiedSideEffects(BridgedInstruction inst);
351351
bool swift_mayAccessPointer(BridgedInstruction inst);
352+
bool swift_mayLoadWeakOrUnowned(BridgedInstruction inst);
353+
bool swift_maySynchronizeNotConsideringSideEffects(BridgedInstruction inst);
354+
bool swift_mayBeDeinitBarrierNotConsideringSideEffects(BridgedInstruction inst);
352355

353356
BridgedInstruction MultiValueInstResult_getParent(BridgedMultiValueResult result);
354357
SwiftInt MultiValueInstResult_getIndex(BridgedMultiValueResult result);

lib/SIL/Utils/MemAccessUtils.cpp

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -418,6 +418,41 @@ bool swift_mayAccessPointer(BridgedInstruction inst) {
418418
return mayAccessPointer(castToInst(inst));
419419
}
420420

421+
bool swift::mayLoadWeakOrUnowned(SILInstruction *instruction) {
422+
return isa<LoadWeakInst>(instruction)
423+
|| isa<LoadUnownedInst>(instruction)
424+
|| isa<StrongCopyUnownedValueInst>(instruction)
425+
|| isa<StrongCopyUnmanagedValueInst>(instruction);
426+
}
427+
428+
bool swift_mayLoadWeakOrUnowned(BridgedInstruction inst) {
429+
return mayLoadWeakOrUnowned(castToInst(inst));
430+
}
431+
432+
/// Conservatively, whether this instruction could involve a synchronization
433+
/// point like a memory barrier, lock or syscall.
434+
bool swift::maySynchronizeNotConsideringSideEffects(SILInstruction *instruction) {
435+
return FullApplySite::isa(instruction)
436+
|| isa<EndApplyInst>(instruction)
437+
|| isa<AbortApplyInst>(instruction);
438+
}
439+
440+
bool swift_maySynchronizeNotConsideringSideEffects(BridgedInstruction inst) {
441+
return maySynchronizeNotConsideringSideEffects(castToInst(inst));
442+
}
443+
444+
bool swift::mayBeDeinitBarrierNotConsideringSideEffects(SILInstruction *instruction) {
445+
bool retval = mayAccessPointer(instruction)
446+
|| mayLoadWeakOrUnowned(instruction)
447+
|| maySynchronizeNotConsideringSideEffects(instruction);
448+
assert(!retval || !isa<BranchInst>(instruction) && "br as deinit barrier!?");
449+
return retval;
450+
}
451+
452+
bool swift_mayBeDeinitBarrierNotConsideringSideEffects(BridgedInstruction inst) {
453+
return mayBeDeinitBarrierNotConsideringSideEffects(castToInst(inst));
454+
}
455+
421456
//===----------------------------------------------------------------------===//
422457
// MARK: AccessRepresentation
423458
//===----------------------------------------------------------------------===//

lib/SILOptimizer/Analysis/BasicCalleeAnalysis.cpp

Lines changed: 1 addition & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -387,30 +387,10 @@ getMemoryBehavior(ApplySite as, bool observeRetains) {
387387
return SILInstruction::MemoryBehavior::MayHaveSideEffects;
388388
}
389389

390-
/// Implementation of mayBeDeinitBarrierNotConsideringSideEffects for use only
391-
/// during bootstrapping or in compilers that lack Swift sources.
392-
///
393-
/// TODO: Remove.
394-
static bool
395-
isDeinitBarrierWithoutEffectsCpp(SILInstruction *const instruction) {
396-
auto mayLoadWeakOrUnowned = [](SILInstruction *const instruction) {
397-
return isa<LoadWeakInst>(instruction) ||
398-
isa<LoadUnownedInst>(instruction) ||
399-
isa<StrongCopyUnownedValueInst>(instruction) ||
400-
isa<StrongCopyUnmanagedValueInst>(instruction);
401-
};
402-
auto maySynchronize = [](SILInstruction *const instruction) {
403-
return isa<FullApplySite>(instruction) || isa<EndApplyInst>(instruction) ||
404-
isa<AbortApplyInst>(instruction);
405-
};
406-
return mayAccessPointer(instruction) || mayLoadWeakOrUnowned(instruction) ||
407-
maySynchronize(instruction);
408-
}
409-
410390
bool swift::isDeinitBarrier(SILInstruction *const instruction,
411391
BasicCalleeAnalysis *bca) {
412392
if (!instructionIsDeinitBarrierFunction) {
413-
return isDeinitBarrierWithoutEffectsCpp(instruction);
393+
return mayBeDeinitBarrierNotConsideringSideEffects(instruction);
414394
}
415395
BridgedInstruction inst = {
416396
cast<SILNode>(const_cast<SILInstruction *>(instruction))};

lib/SILOptimizer/Utils/ShrinkBorrowScope.cpp

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -384,6 +384,15 @@ bool Rewriter::run() {
384384
if (auto *terminator = dyn_cast<TermInst>(instruction)) {
385385
auto successors = terminator->getParentBlock()->getSuccessorBlocks();
386386
for (auto *successor : successors) {
387+
// If a terminator is a barrier, it must not branch to a merge point.
388+
// Doing so would require one of the following:
389+
// - the terminator was passed a phi--which is handled by barriers.phis
390+
// - the terminator had a result--which can't happen thanks to the lack
391+
// of critical edges
392+
// - the terminator was a BranchInst which was passed no arguments but
393+
// which was nonetheless identified as a barrier--which is illegal
394+
assert(successor->getSinglePredecessorBlock() ==
395+
terminator->getParentBlock());
387396
madeChange |= createEndBorrow(&successor->front());
388397
}
389398
} else {
@@ -400,10 +409,11 @@ bool Rewriter::run() {
400409
// don't have multiple predecessors) whose end was not reachable (because
401410
// reachability was not able to make it to the top of some other successor).
402411
//
403-
// In other words, a control flow boundary is the target edge from a block B
404-
// to its single predecessor P not all of whose successors S in succ(P) had
405-
// reachable beginnings. We witness that fact about P's successors by way of
406-
// P not having a reachable end--see BackwardReachability::meetOverSuccessors.
412+
// In other words, a control flow boundary is the target block of the edge
413+
// to a block B from its single predecessor P not all of whose successors S
414+
// in succ(P) had reachable beginnings. We witness that fact about P's
415+
// successors by way of P not having a reachable end--see
416+
// IterativeBackwardReachability::meetOverSuccessors.
407417
//
408418
// control-flow-boundary(B) := beginning-reachable(B) && !end-reachable(P)
409419
for (auto *block : barriers.blocks) {

0 commit comments

Comments
 (0)