Skip to content

Commit 225c3f8

Browse files
committed
[semantic-arc] Rename isConsumed -> isDeadLiveRange.
Also, eliminate some bitrot from comments.
1 parent 5085e45 commit 225c3f8

File tree

1 file changed

+19
-18
lines changed

1 file changed

+19
-18
lines changed

lib/SILOptimizer/Mandatory/SemanticARCOpts.cpp

Lines changed: 19 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -39,11 +39,12 @@ STATISTIC(NumLoadCopyConvertedToLoadBorrow,
3939
// Utility
4040
//===----------------------------------------------------------------------===//
4141

42-
/// Return true if v only has invalidating uses that are destroy_value.
42+
/// Return true if v only has invalidating uses that are destroy_value. Such an
43+
/// owned value is said to represent a dead "live range".
4344
///
4445
/// Semantically this implies that a value is never passed off as +1 to memory
4546
/// or another function implying it can be used everywhere at +0.
46-
static bool isConsumed(
47+
static bool isDeadLiveRange(
4748
SILValue v, SmallVectorImpl<DestroyValueInst *> &destroys,
4849
NullablePtr<SmallVectorImpl<SILInstruction *>> forwardingInsts = nullptr) {
4950
assert(v.getOwnershipKind() == ValueOwnershipKind::Owned);
@@ -99,8 +100,9 @@ static bool isConsumed(
99100
continue;
100101
}
101102

102-
// Otherwise be conservative and assume that we /may consume/ the value.
103-
return true;
103+
// Otherwise be conservative and assume that we /may consume/ the value,
104+
// so the live range must not be eliminated.
105+
return false;
104106
}
105107
case UseLifetimeConstraint::MustBeLive:
106108
// Ok, this constraint can take something owned as live. Assert that it
@@ -114,7 +116,9 @@ static bool isConsumed(
114116
}
115117
}
116118

117-
return false;
119+
// We visited all of our users and were able to prove that all of them were
120+
// benign. Return true.
121+
return true;
118122
}
119123

120124
//===----------------------------------------------------------------------===//
@@ -332,17 +336,14 @@ bool SemanticARCOptVisitor::performGuaranteedCopyValueOptimization(CopyValueInst
332336
if (!canHandleOperand(cvi->getOperand(), borrowIntroducers))
333337
return false;
334338

335-
// Then go over all of our uses. Find our destroying instructions (ignoring
336-
// forwarding instructions that can forward both owned and guaranteed) and
337-
// make sure all of them are destroy_value. For our non-destroying
338-
// instructions, make sure that they accept a guaranteed value. After that,
339-
// make sure that our destroys are within the lifetime of our borrowed values.
340-
//
341-
// TODO: Change isConsumed to return branch propagated users for destroys, so
342-
// we do not need to construct another array.
339+
// Then go over all of our uses and see if the value returned by our copy
340+
// value forms a dead live range. If we do not have a dead live range, there
341+
// must be some consuming use that we either do not understand is /actually/
342+
// forwarding or a user that truly represents a necessary consume of the
343+
// value (e.x. storing into memory).
343344
SmallVector<DestroyValueInst *, 16> destroys;
344345
SmallVector<SILInstruction *, 16> guaranteedForwardingInsts;
345-
if (isConsumed(cvi, destroys, &guaranteedForwardingInsts))
346+
if (!isDeadLiveRange(cvi, destroys, &guaranteedForwardingInsts))
346347
return false;
347348

348349
// Next check if we have any destroys at all of our copy_value and an operand
@@ -724,15 +725,15 @@ bool SemanticARCOptVisitor::visitLoadInst(LoadInst *li) {
724725
if (li->getOwnershipQualifier() != LoadOwnershipQualifier::Copy)
725726
return false;
726727

727-
// Ok, we have our load [copy]. Make sure its value is never
728-
// consumed. If it is consumed, we need to pass off a +1 value, so
729-
// bail.
728+
// Ok, we have our load [copy]. Make sure its value is truly a dead live range
729+
// implying it is only ever consumed by destroy_value instructions. If it is
730+
// consumed, we need to pass off a +1 value, so bail.
730731
//
731732
// FIXME: We should consider if it is worth promoting a load [copy]
732733
// -> load_borrow if we can put a copy_value on a cold path and thus
733734
// eliminate RR traffic on a hot path.
734735
SmallVector<DestroyValueInst *, 32> destroyValues;
735-
if (isConsumed(li, destroyValues))
736+
if (!isDeadLiveRange(li, destroyValues))
736737
return false;
737738

738739
// Then check if our address is ever written to. If it is, then we

0 commit comments

Comments
 (0)