[SemanticARCOpts] Don't shorten owned lexical values lifetimes through deinit barriers.

[nate-chandler]

Prevent joining lifetimes of copies with destroyed owned lexical values if there may be deinit barriers between the final consume of the copy and the destroy of the lexical value.

rdar://108014714

[nate-chandler]

@swift-ci please test

[nate-chandler]

@swift-ci please benchmark

[meg-gupta]

LGTM!

Looks like other edge cases in tryJoiningCopyValueLiveRangeWithOperand may also require a similar fix.

[nate-chandler]

@meg-gupta Thank you!

Here's an attempt to list all the places where tryJoiningCopyValueLiveRangeWithOperand (or its callees) returns true and eliminates a copy. For each case, there's an idea for why eliminating the copy doesn't shorten the lifetime.

There was quite a bit to look at, so I may have missed something. Are there other cases than these or is there trouble in any of these cases?

%cvi = copy_value %original
destroy_value %original // dvi

SemanticARCOptVisitor::tryJoiningCopyValueLiveRangeWithOperand
- tryJoiningIfCopyOperandHasSingleDestroyValue
  - canJoinIfCopyDiesInFunctionExitingBlock
    - consumingUser(cvi) == term(Exit)
      Idea: cvi's lifetime ends at function exit.  original's lifetime must be shorter.
    - consumingUser(cvi) ∈ Exit && dvi ∉ Exit
      Idea: cvi's lifetime ends "near" function exit; original's lifetime does not, i.e. it ends sometime earlier.  original's lifetime must be shorter.
  - tryJoinIfDestroyConsumingUseInSameBlock
    - Fixed in this patch.
- block(cvi) == block(dvi) && count(consumingUsers(cvi)) != 1 
  Idea: Either count(consumingUsers(cvi)) == 0 or count(consumingUsers(cvi)) > 1
        Suppose count(consumingUsers(cvi)) == 0.  Then cvi's lifetime ends at unreachables (although with complete lifetimes this can't happen).  original's lifetime ends at dvi, somewhere before that.  original's lifetime must be shorter.
        Suppose count(consumingUsers(cvi)) > 1.  Then cvi must be live-out of block.  And original's lifetime ends at dvi, in block.  original's lifetime must be shorter.
- block(cvi) == block(dvi) && block(consumingUser(cvi)) != block(cvi) 
  Idea: cvi's lifetime ends in some other block.  cvi is live-out of block.  original ends at dvi in block.  original's lifetime must be shorter.
- tryJoinIfDestroyConsumingUseInSameBlock
  - Fixed in this patch.

[nate-chandler]

------- Performance (x86_64): -O -------

IMPROVEMENT                 OLD      NEW      DELTA    RATIO
FlattenListLoop             1619.0   983.5    -39.3%   **1.65x (?)**
ArrayAppendGenericStructs   1780.0   1362.5   -23.5%   **1.31x (?)**

------- Code size: -O -------

------- Performance (x86_64): -Osize -------

REGRESSION                  OLD        NEW      DELTA    RATIO
FlattenListFlatMap          2817.0     4562.0   +61.9%   **0.62x (?)**

IMPROVEMENT                 OLD        NEW      DELTA    RATIO
ArrayAppendGenericStructs   1753.333   1250.0   -28.7%   **1.40x (?)**

------- Code size: -Osize -------

------- Performance (x86_64): -Onone -------

REGRESSION                  OLD      NEW      DELTA    RATIO
ArrayAppendGenericStructs   1232.0   1405.0   +14.0%   **0.88x (?)**

------- Code size: -swiftlibs -------

[meg-gupta]

@nate-chandler Thank you! I went through everything as well, and looks like we don't shorten lifetimes anywhere else.

[meg-gupta]

Although, I wonder if we should also check for interior pointers here and other places we check for pointer escapes (sigh!) - https://github.com/apple/swift/blob/42d2441183e674d7bf4058bb101773d502603f2e/lib/SILOptimizer/SemanticARC/CopyValueOpts.cpp#L499. I didn't write any test, but we handle interior pointers conservatively in CanonicalizeOSSALifetimes for similar reasons..

[nate-chandler]

Interesting--that sounds like it could be a good follow-on to #64836 where the hasPointerEscape check was added. I'll look into that next.

[atrick]

LGTM