Fix CSE isEqual and HashVisitor for escaped values
#63791
Conversation
|
@swift-ci test |
|
@swift-ci benchmark |
|
@swift-ci Please Apple Silicon benchmark |
| bool isIdenticalTo( | ||
| const SILInstruction *RHS, | ||
| std::function<bool(const Operand *, const Operand *)> opEqual) const { | ||
| // Quick check if both instructions have the same kind, number of operands, |
There was a problem hiding this comment.
TODO: Refactor code common to bool isIdenticalTo(const SILInstruction *RHS, std::function<bool(SILValue, SILValue)> opEqual) const
|
@swift-ci test |
|
@swift-ci benchmark |
| auto opOwnership = op->getOperandOwnership(); | ||
|
|
||
| if (opOwnership == OperandOwnership::PointerEscape || | ||
| opOwnership == OperandOwnership::BitwiseEscape || |
There was a problem hiding this comment.
I think we need a comment here because we don't normally consider BitwiseEscape to mean we can't optimize the lifetime. This looks like the usual check for "can we optimize this lifetime" but I think it's actually different. Here you're trying to catch all the cases that RAUW doesn't handle correctly, right? Can we just check exactly the same condition that RAUW checks and comment that they need to be in-sync?
There was a problem hiding this comment.
Unfortunately, there isn't any specific function that OSSA RAUW checks for. It doesn't bailout for none, unowned values, it replaces all uses with the replacement value without checking for lifetime.
|
@swift-ci benchmark |
|
@swift-ci test |
meg-gupta
force-pushed
the
csebug
branch
2 times, most recently
from
608fc81 to
5d5773e
Compare
|
@swift-ci benchmark |
|
@swift-ci apple silicon benchmark |
meg-gupta
changed the title
isEqual and HashVisitor for non-guaranteed, non-owned valuesisEqual and HashVisitor for escaped values
|
@swift-ci test |
|
@swift-ci test |
|
@swift-ci test |
|
@swift-ci test linux platform |
1 similar comment
|
@swift-ci test linux platform |
CSE relies on OSSA RAUW for lifetime extension when replacing a redundant instruction. OSSA RAUW however does not handle lifetime extension for escaped base values. Values escape from ownership via pointer escape, bitwise escape, forwarding unowned operations and have none or unowned ownership. For all such values do not look through ownership instructions while determining equality. It is possible to CSE such values with equivalent operands, because the operand use guarantees lifetime of the base operand.
|
@swift-ci test and merge |
CSE would look through ownership instructions while matching operands, this isn't valid in the case of escaped values. CSE relies on OSSA RAUW for replacing the redundant instruction which extends lifetimes when necessary for replacement. For escaped values, OSSA RAUW does not extend the base value lifetime. For all such instructions, this PR changes CSE to not look through ownership instructions.
This PR also fixes non-determinism in CSE for ref_to_unmanaged instruction, before the changes in the PR,
llvm::DenseMapInfo<SimpleValue>::isEqualwas looking through ownership instructions while matching operands, butHashVisitor::visit_ref_to_unmanageddid not.Fixes rdar://105627656