Skip to content

Commit 9af40b3

Browse files
committed
[LexicalDestroyHoisting] Eliminate updateSSA.
UpdateSSA was being used to rewrite destroys, not for its intended purpose. Besides being a misuse, it's also wasteful. Instead, just rewrite the destroys that it would have updated.
1 parent a541c83 commit 9af40b3

File tree

2 files changed

+36
-43
lines changed

2 files changed

+36
-43
lines changed

lib/SILOptimizer/Utils/LexicalDestroyFolding.cpp

Lines changed: 19 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -308,9 +308,6 @@ class FilterCandidates final {
308308

309309
/// Determines whether each candidate is viable for folding.
310310
///
311-
/// Requiring findUsers to have been called, this uses the more expensive
312-
/// isViableMatch.
313-
///
314311
/// returns true if any candidates were viable
315312
/// false otherwise
316313
bool run(Candidates &candidates);
@@ -334,8 +331,7 @@ class FilterCandidates final {
334331
///
335332
/// If there are, we can't fold the apply. Specifically, we can't introduce
336333
/// a move_value [lexical] %borrowee because that value still needs to be used
337-
/// in those locations. While updateSSA would happily rewrite those uses to
338-
/// be uses of the move, that is not a semantically valid transformation.
334+
/// in those locations.
339335
///
340336
/// For example, given the following SIL
341337
/// %borrowee : @owned
@@ -412,10 +408,6 @@ class Rewriter final {
412408
/// (3) delete the destroy_value
413409
void fold(Match, SmallVectorImpl<int> const &rewritableArgumentIndices);
414410

415-
// Update SSA to reflect that fact that %move and %borrowee are two
416-
// defs for the "same value".
417-
void updateSSA();
418-
419411
// The move_value [lexical] instruction that was added during the run.
420412
//
421413
// Defined during createMove.
@@ -470,25 +462,31 @@ bool run(Context &context) {
470462
void Rewriter::run() {
471463
bool foldedAny = false;
472464
(void)foldedAny;
473-
for (unsigned index = 0, count = candidates.vector.size(); index < count;
474-
++index) {
465+
auto size = candidates.vector.size();
466+
for (unsigned index = 0; index < size; ++index) {
475467
auto candidate = candidates.vector[index];
476-
if (!candidate.viable)
477-
continue;
478468
createMove();
469+
if (!candidate.viable) {
470+
// Nonviable candidates still end with the pattern
471+
//
472+
// end_borrow %lifetime
473+
// ...
474+
// destroy_value %borrowee
475+
//
476+
// Now that the new move_value [lexical] dominates all candidates, the
477+
// every candidate's destroy_value %borrowee is dominated by it, so every
478+
// one is dominated by another consuming use which is illegal. Rewrite
479+
// each such destroy_value to be a destroy_value of the move.
480+
candidate.match.dvi->setOperand(mvi);
481+
continue;
482+
}
479483

480484
fold(candidate.match, candidate.argumentIndices);
481485
#ifndef NDEBUG
482486
foldedAny = true;
483487
#endif
484488
}
485489
assert(foldedAny && "rewriting without anything to rewrite!?");
486-
487-
// There are now "two defs for %borrowee":
488-
// - %borrowee
489-
// - %move
490-
// We need to update SSA.
491-
updateSSA();
492490
}
493491

494492
bool Rewriter::createMove() {
@@ -551,26 +549,6 @@ void Rewriter::fold(Match candidate,
551549
context.deleter.forceDelete(candidate.dvi);
552550
}
553551

554-
void Rewriter::updateSSA() {
555-
SILSSAUpdater updater;
556-
updater.initialize(context.borrowee->getType(),
557-
context.borrowee.getOwnershipKind());
558-
updater.addAvailableValue(context.borrowee->getParentBlock(),
559-
context.borrowee);
560-
updater.addAvailableValue(mvi->getParentBlock(), mvi);
561-
562-
SmallVector<Operand *, 16> uses;
563-
for (auto use : context.borrowee->getUses()) {
564-
if (use->getUser() == mvi)
565-
continue;
566-
uses.push_back(use);
567-
}
568-
569-
for (auto use : uses) {
570-
updater.rewriteUse(*use);
571-
}
572-
}
573-
574552
//===----------------------------------------------------------------------===//
575553
// MARK: Lookups
576554
//===----------------------------------------------------------------------===//
@@ -640,6 +618,8 @@ bool FilterCandidates::run(Candidates &candidates) {
640618

641619
bool findBorroweeUsage(Context &context, BorroweeUsage &usage) {
642620
auto recordUse = [&](Operand *use) {
621+
// Ignore uses that aren't dominated by the introducer. PrunedLiveness
622+
// relies on us doing this check.
643623
if (!context.dominanceTree.dominates(context.introducer, use->getUser()))
644624
return;
645625
usage.uses.push_back(use);

test/SILOptimizer/lexical_destroy_folding.sil

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -161,7 +161,7 @@ exit:
161161

162162
}
163163

164-
// Enclose a single guaranteed lexical scope that contains two paralell
164+
// Enclose a single guaranteed lexical scope that contains two parallel
165165
// applies of owned arguments with the destroy at a NONcanonical position
166166
// within a single owned lexical scope.
167167
//
@@ -514,11 +514,24 @@ exit:
514514
return %retval : $()
515515
}
516516

517-
// Don't fold with apply when guaranteed lexical value used in one but not two
518-
// branches and the lexical scope ends before the use on the non-lexical branch.
517+
// Fold apply when guaranteed lexical value used in one but not two branches
518+
// and the lexical scope ends before the use on the non-lexical branch.
519519
//
520520
// CHECK-LABEL: sil [ossa] @nofold_two_parallel_owned_uses_one_lexical___scope_ends_before_use : {{.*}} {
521-
// CHECK-NOT: move_value [lexical]
521+
// CHECK: {{bb[0-9]+}}([[INSTANCE:%[^,]+]] :
522+
// CHECK: [[COPY:%[^,]+]] = copy_value [[INSTANCE]]
523+
// CHECK: [[MOVE:%[^,]+]] = move_value [lexical] [[INSTANCE]]
524+
// CHECK: [[CALLEE_OWNED:%[^,]+]] = function_ref @callee_owned
525+
// CHECK: cond_br undef, [[LEFT:bb[0-9]+]], [[RIGHT:bb[0-9]+]]
526+
// CHECK: [[LEFT]]:
527+
// CHECK: apply [[CALLEE_OWNED]]([[MOVE]])
528+
// CHECK: destroy_value [[COPY]]
529+
// CHECK: br [[EXIT:bb[0-9]+]]
530+
// CHECK: [[RIGHT]]:
531+
// CHECK: destroy_value [[MOVE]]
532+
// CHECK: apply [[CALLEE_OWNED]]([[COPY]])
533+
// CHECK: br [[EXIT]]
534+
// CHECK: [[EXIT]]:
522535
// CHECK-LABEL: } // end sil function 'nofold_two_parallel_owned_uses_one_lexical___scope_ends_before_use'
523536
sil [ossa] @nofold_two_parallel_owned_uses_one_lexical___scope_ends_before_use : $@convention(thin) (@owned C) -> () {
524537
entry(%instance : @owned $C):

0 commit comments

Comments
 (0)