Skip to content

Commit df6ff7f

Browse files
committed
[semantic-arc-opts] Teach load [copy] -> load_borrow about forwarding instructions.
I implemented this some time ago for copy_value, but I (IIRC due to time) did not enable it for the load [copy] optimization. Specifically now we should be able to optimize this: ``` %ref = ref_element_addr %guaranteedArg %0a = load [copy] %ref %0b = unchecked_ref_cast %0a destroy_value %0b ``` -> ``` %ref = ref_element_addr %guaranteedArg %0a = load_borrow %guaranteedArg %0b = unchecked_ref_cast %0a end_borrow %0a ```
1 parent bcf93c6 commit df6ff7f

File tree

2 files changed

+76
-48
lines changed

2 files changed

+76
-48
lines changed

lib/SILOptimizer/Mandatory/SemanticARCOpts.cpp

Lines changed: 54 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -44,9 +44,9 @@ STATISTIC(NumLoadCopyConvertedToLoadBorrow,
4444
///
4545
/// Semantically this implies that a value is never passed off as +1 to memory
4646
/// or another function implying it can be used everywhere at +0.
47-
static bool isDeadLiveRange(
48-
SILValue v, SmallVectorImpl<DestroyValueInst *> &destroys,
49-
NullablePtr<SmallVectorImpl<SILInstruction *>> forwardingInsts = nullptr) {
47+
static bool
48+
isDeadLiveRange(SILValue v, SmallVectorImpl<DestroyValueInst *> &destroys,
49+
SmallVectorImpl<SILInstruction *> &forwardingInsts) {
5050
assert(v.getOwnershipKind() == ValueOwnershipKind::Owned);
5151
SmallVector<Operand *, 32> worklist(v->use_begin(), v->use_end());
5252
while (!worklist.empty()) {
@@ -84,8 +84,7 @@ static bool isDeadLiveRange(
8484
//
8585
// NOTE: Today we do not support TermInsts for simplicity... we /could/
8686
// support it though if we need to.
87-
if (forwardingInsts.isNull() || isa<TermInst>(user) ||
88-
!isGuaranteedForwardingInst(user) ||
87+
if (isa<TermInst>(user) || !isGuaranteedForwardingInst(user) ||
8988
1 != count_if(user->getOperandValues(
9089
true /*ignore type dependent operands*/),
9190
[&](SILValue v) {
@@ -98,7 +97,7 @@ static bool isDeadLiveRange(
9897
// Ok, this is a forwarding instruction whose ownership we can flip from
9998
// owned -> guaranteed. Visit its users recursively to see if the the
10099
// users force the live range to be alive.
101-
forwardingInsts.get()->push_back(user);
100+
forwardingInsts.push_back(user);
102101
for (SILValue v : user->getResults()) {
103102
if (v.getOwnershipKind() != ValueOwnershipKind::Owned)
104103
continue;
@@ -298,6 +297,49 @@ static bool canHandleOperand(SILValue operand, SmallVectorImpl<SILValue> &out) {
298297
return all_of(out, canHandleValue);
299298
}
300299

300+
static void convertForwardingInstsFromOwnedToGuaranteed(
301+
SmallVectorImpl<SILInstruction *> &guaranteedForwardingInsts) {
302+
// Then change all of our guaranteed forwarding insts to have guaranteed
303+
// ownership kind instead of what ever they previously had (ignoring trivial
304+
// results);
305+
while (!guaranteedForwardingInsts.empty()) {
306+
auto *i = guaranteedForwardingInsts.pop_back_val();
307+
assert(i->hasResults());
308+
309+
for (SILValue result : i->getResults()) {
310+
if (auto *svi = dyn_cast<OwnershipForwardingSingleValueInst>(result)) {
311+
if (svi->getOwnershipKind() == ValueOwnershipKind::Owned) {
312+
svi->setOwnershipKind(ValueOwnershipKind::Guaranteed);
313+
}
314+
continue;
315+
}
316+
317+
if (auto *ofci = dyn_cast<OwnershipForwardingConversionInst>(result)) {
318+
if (ofci->getOwnershipKind() == ValueOwnershipKind::Owned) {
319+
ofci->setOwnershipKind(ValueOwnershipKind::Guaranteed);
320+
}
321+
continue;
322+
}
323+
324+
if (auto *sei = dyn_cast<OwnershipForwardingSelectEnumInstBase>(result)) {
325+
if (sei->getOwnershipKind() == ValueOwnershipKind::Owned) {
326+
sei->setOwnershipKind(ValueOwnershipKind::Guaranteed);
327+
}
328+
continue;
329+
}
330+
331+
if (auto *mvir = dyn_cast<MultipleValueInstructionResult>(result)) {
332+
if (mvir->getOwnershipKind() == ValueOwnershipKind::Owned) {
333+
mvir->setOwnershipKind(ValueOwnershipKind::Guaranteed);
334+
}
335+
continue;
336+
}
337+
338+
llvm_unreachable("unhandled forwarding instruction?!");
339+
}
340+
}
341+
}
342+
301343
// Eliminate a copy of a borrowed value, if:
302344
//
303345
// 1. All of the copies users do not consume the copy (and thus can accept a
@@ -345,7 +387,7 @@ bool SemanticARCOptVisitor::performGuaranteedCopyValueOptimization(CopyValueInst
345387
// value (e.x. storing into memory).
346388
SmallVector<DestroyValueInst *, 16> destroys;
347389
SmallVector<SILInstruction *, 16> guaranteedForwardingInsts;
348-
if (!isDeadLiveRange(cvi, destroys, &guaranteedForwardingInsts))
390+
if (!isDeadLiveRange(cvi, destroys, guaranteedForwardingInsts))
349391
return false;
350392

351393
// Next check if we have any destroys at all of our copy_value and an operand
@@ -452,47 +494,8 @@ bool SemanticARCOptVisitor::performGuaranteedCopyValueOptimization(CopyValueInst
452494
}
453495

454496
eraseAndRAUWSingleValueInstruction(cvi, cvi->getOperand());
497+
convertForwardingInstsFromOwnedToGuaranteed(guaranteedForwardingInsts);
455498

456-
// Then change all of our guaranteed forwarding insts to have guaranteed
457-
// ownership kind instead of what ever they previously had (ignoring trivial
458-
// results);
459-
while (!guaranteedForwardingInsts.empty()) {
460-
auto *i = guaranteedForwardingInsts.pop_back_val();
461-
462-
assert(i->hasResults());
463-
464-
for (SILValue result : i->getResults()) {
465-
if (auto *svi = dyn_cast<OwnershipForwardingSingleValueInst>(result)) {
466-
if (svi->getOwnershipKind() == ValueOwnershipKind::Owned) {
467-
svi->setOwnershipKind(ValueOwnershipKind::Guaranteed);
468-
}
469-
continue;
470-
}
471-
472-
if (auto *ofci = dyn_cast<OwnershipForwardingConversionInst>(result)) {
473-
if (ofci->getOwnershipKind() == ValueOwnershipKind::Owned) {
474-
ofci->setOwnershipKind(ValueOwnershipKind::Guaranteed);
475-
}
476-
continue;
477-
}
478-
479-
if (auto *sei = dyn_cast<OwnershipForwardingSelectEnumInstBase>(result)) {
480-
if (sei->getOwnershipKind() == ValueOwnershipKind::Owned) {
481-
sei->setOwnershipKind(ValueOwnershipKind::Guaranteed);
482-
}
483-
continue;
484-
}
485-
486-
if (auto *mvir = dyn_cast<MultipleValueInstructionResult>(result)) {
487-
if (mvir->getOwnershipKind() == ValueOwnershipKind::Owned) {
488-
mvir->setOwnershipKind(ValueOwnershipKind::Guaranteed);
489-
}
490-
continue;
491-
}
492-
493-
llvm_unreachable("unhandled forwarding instruction?!");
494-
}
495-
}
496499
++NumEliminatedInsts;
497500
return true;
498501
}
@@ -735,7 +738,8 @@ bool SemanticARCOptVisitor::visitLoadInst(LoadInst *li) {
735738
// -> load_borrow if we can put a copy_value on a cold path and thus
736739
// eliminate RR traffic on a hot path.
737740
SmallVector<DestroyValueInst *, 32> destroyValues;
738-
if (!isDeadLiveRange(li, destroyValues))
741+
SmallVector<SILInstruction *, 16> guaranteedForwardingInsts;
742+
if (!isDeadLiveRange(li, destroyValues, guaranteedForwardingInsts))
739743
return false;
740744

741745
// Then check if our address is ever written to. If it is, then we
@@ -760,6 +764,8 @@ bool SemanticARCOptVisitor::visitLoadInst(LoadInst *li) {
760764
}
761765

762766
eraseAndRAUWSingleValueInstruction(li, lbi);
767+
convertForwardingInstsFromOwnedToGuaranteed(guaranteedForwardingInsts);
768+
763769
++NumEliminatedInsts;
764770
++NumLoadCopyConvertedToLoadBorrow;
765771
return true;

test/SILOptimizer/semantic-arc-opts.sil

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -437,6 +437,28 @@ bb0(%x : @guaranteed $ClassLet):
437437
return undef : $()
438438
}
439439

440+
// CHECK-LABEL: sil [ossa] @dont_copy_let_properties_with_guaranteed_base_and_forwarding_uses :
441+
// CHECK: ref_element_addr
442+
// CHECK-NEXT: load_borrow
443+
// CHECK-NEXT: unchecked_ref_cast
444+
// CHECK-NEXT: apply
445+
// CHECK-NEXT: end_borrow
446+
// CHECK-NEXT: return
447+
// CHECK: } // end sil function 'dont_copy_let_properties_with_guaranteed_base_and_forwarding_uses'
448+
sil [ossa] @dont_copy_let_properties_with_guaranteed_base_and_forwarding_uses : $@convention(thin) (@guaranteed ClassLet) -> () {
449+
bb0(%x : @guaranteed $ClassLet):
450+
%f = function_ref @black_hole : $@convention(thin) (@guaranteed Klass) -> ()
451+
452+
%p = ref_element_addr %x : $ClassLet, #ClassLet.aLet
453+
%v = load [copy] %p : $*Klass
454+
%c = unchecked_ref_cast %v : $Klass to $Klass
455+
%b = begin_borrow %c : $Klass
456+
apply %f(%b) : $@convention(thin) (@guaranteed Klass) -> ()
457+
end_borrow %b : $Klass
458+
destroy_value %c : $Klass
459+
return undef : $()
460+
}
461+
440462
// CHECK-LABEL: sil [ossa] @dont_copy_let_properties_with_guaranteed_upcast_base
441463
// CHECK: ref_element_addr
442464
// CHECK-NEXT: load_borrow

0 commit comments

Comments
 (0)