Skip to content

Commit 7176aaf

Browse files
committed
Add a comment to TempRValue optimization.
A recent fix added some code that is inconsistent with the design of the pass, creates an implicit coupling between separate parts of the optimization, and relies on incredibly subtle reasoning to ensure correctness. This sort of thing needs a big fat comment.
1 parent 7e3357a commit 7176aaf

File tree

1 file changed

+12
-2
lines changed

1 file changed

+12
-2
lines changed

lib/SILOptimizer/Transforms/CopyForwarding.cpp

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1659,8 +1659,18 @@ bool TempRValueOptPass::collectLoads(
16591659

16601660
// Check if there is another function argument, which is inout which might
16611661
// modify the source of the copy_addr.
1662-
// If we would remove the copy_addr in this case, it would result in an
1663-
// exclusivity violation.
1662+
//
1663+
// When a use of the temporary is an apply, then we need to prove that the
1664+
// function called by the apply cannot modify the temporary's source
1665+
// value. By design, this should be handled by
1666+
// `checkNoSourceModification`. However, this would be too conservative
1667+
// since it's common for the apply to have an @out argument, and alias
1668+
// analysis cannot prove that the @out does not alias with `src`. Instead,
1669+
// `checkNoSourceModification` always avoids analyzing the current use, so
1670+
// applies need to be handled here. We already know that an @out cannot
1671+
// alias with `src` because the `src` value must be initialized at the point
1672+
// of the call. Hence, it is sufficient to check specifically for another
1673+
// @inout that might alias with `src`.
16641674
auto calleeConv = apply.getSubstCalleeConv();
16651675
unsigned calleeArgIdx = apply.getCalleeArgIndexOfFirstAppliedArg();
16661676
for (Operand &operand : apply.getArgumentOperands()) {

0 commit comments

Comments
 (0)