Skip to content

Conversation

@eeckstein
Copy link
Contributor

@eeckstein eeckstein commented Dec 17, 2025

  • handle debug_value in begin_borrow simplification
  • ignore type-dependent operands when converting borrowed -> owned
  • remove borrow scopes which are borrowing an already guaranteed value
  • allow optimizing lexical begin_borrows outside the mandatory pipeline
  • SimplifyLoad/SimplifyLoadBorrow: replace address casts of heap objects
  • convert destructure_tuple/destructure_struct with guaranteed ownership to to tuple_extract/struct_extract

The improvements for begin_borrow simplification allows to remove the now obsolete BorrowScope optimization from SemanticArcOpts.

This PR also fixes a few small bugs which I run into while working on the optimizations:

  • don't remove begin_borrow [lexical] of a thin_to_thick_function in the mandatory pipeline
  • OwnershipOptUtils: fix a wrong debug location when creating destroy_value
  • CopyToBorrowOptimization: need to update borrowed-from instructions when changes are made

For details see the commit messages

* rename `lookThroughSingleForwardingUses` -> `lookThroughOwnedConvertibaleForwardingChain`
* fix the comment of `replaceGuaranteed`
When trying to remove a borrow scope by replacing all guaranteed uses with owned uses, don't bail for values which have debug_value uses.
Also make sure that the debug_value instructions are located within the owned value's lifetime.
* remove borrow scopes which are borrowing an already guaranteed value
* allow optimizing lexical `begin_borrows` outside the mandatory pipeline
* fix: don't remove `begin_borrow [lexical]` of a `thin_to_thick_function` in the mandatory pipeline
…alue

When taking the location from the builder's insertion point, we must make sure it's not a return location.
Fixes an assertion failure.
I found this during my work on simplifying borrow scopes
@eeckstein eeckstein requested a review from jckarter as a code owner December 17, 2025 18:09
Replaces address casts of heap objects
```
  %1 = unchecked_addr_cast %0 : $*SomeClass to $*OtherClass
  %2 = load [copy] %1
```
with ref-casts of the loaded value
```
  %1 = load [copy] %0
  %2 = unchecked_ref_cast %1 : $SomeClass to $OtherClass
```
… with guaranteed ownership to to `tuple_extract`/`struct_extract`
By defining its own array structs, which are independent from objc interop.
Also, the requires-line was wrong anyway. The test didn't run on macos either.
…hen changes are made

When ownership is changed from owned to guaranteed, the enclosing values of a guaranteed value can change.
Fixes a SIL verifier error.
@eeckstein
Copy link
Contributor Author

@swift-ci test

@eeckstein
Copy link
Contributor Author

@swift-ci apple silicon benchmark

@eeckstein eeckstein requested review from aidan-hall, atrick, elsakeirouz and meg-gupta and removed request for jckarter December 17, 2025 18:11
return true
}

private func tryForwardStoreBorrow(_ context: SimplifyContext) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a unit test for store borrow forwarding?

/// ```
/// %1 = load [copy] %0
/// %2 = unchecked_ref_cast %1 : $SomeClass to $OtherClass
/// ```
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you comment why this transform is useful?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants