[AddressLowering] Copy for out-of-range stores. #62609
Conversation
|
@swift-ci please test |
|
@swift-ci please test |
| if (llvm::any_of(roots, [](SILValue root) { | ||
| return isa<BeginApplyInst>(root->getDefiningInstruction()); | ||
| SSAPrunedLiveness liveness; | ||
| if (llvm::any_of(roots, [&liveness, storeInst](SILValue root) { |
There was a problem hiding this comment.
fwiw, this is an example of where I find range-based for loops easier to read (rather than mentally factoring a closure with inverted an return value). Alternatively, you explicitly factor it into something like isStoreCopyRootValid
There was a problem hiding this comment.
ok.... you basically already did that in the following commit
| return true; | ||
| } | ||
| liveness.initializeDef(root); | ||
| auto summary = liveness.computeSimple(); |
There was a problem hiding this comment.
This seems reasonably safe. But in the future we should be more precise and efficient by doing
- visitBorrowIntroducers (instead of roots)
- ExtendedLiveness.compute for each introducer (I haven't posted the PR yet)
That would either find a guaranteed argument and do no work at all, or find a begin_borrow and only visit the end_borrows. It could also handle reborrow-extended liveness.
Up to you if you want a TODO here
There was a problem hiding this comment.
Sounds great! Added a TODO.
Rather than hard-coding an operand number, ask the operand for it.
Move a cheap check that the one user is a store over the expensive check done when the value copied has guaranteed ownership.
When a copy_value's only use is a store, in some cases, as an optimization, creating a copy_addr can be skipped. Whether a value is such a copy_value is determined by the isStoreCopy predicate. If the operand of the copy_value is a guaranteed value and any of its guaranteed roots have live ranges which the store is outside of, skip this optimization and create the copy_addr.
Skip the store-copy optimization if the store instruction occurs outside the lifetime of the value being copied.
nate-chandler
force-pushed
the
opaque-values/2/20221214
branch
from
e934b46 to
356f2b7
Compare
|
@swift-ci please smoke test and merge |
When a
copy_value's only use is astore, in some cases, as an optimization, creating acopy_addrcan be skipped. Whether a value is such acopy_valueis determined by theisStoreCopypredicate.If the operand of the
copy_valueis such that thestoreis outside the relevant live ranges (those of the guaranteed roots or that of the owned value), skip this optimization and create thecopy_addr.