Skip to content

Conversation

@meg-gupta
Copy link
Contributor

Currently borrow accessor SILGen uses unchecked_ownership_conversion instructions to produce a @guaranteed return value from an address in some cases:

  %ld = load_borrow %self
  %ex = struct_extract %ld, #Struct.storedProperty
  %conv1 = unchecked_ownership_conversion %ex to @unowned
  %conv2 = unchecked_ownership_conversion %conv2 to @guaranteed
  end_borrow %ld
  return %conv2

unchecked_ownership_conversion silences the ownership verifier, but is an unsafe SIL operation which will get treated conservatively in the optimizer.

Introduce a new return_borrow instruction and add a new SILGenCleanup transform, to turn this into valid OSSA:

  %ld = load_borrow %self
  %ex = struct_extract %ld, #Struct.storedProperty
  return_borrow %ex from_scopes %ld

return_borrow generation becomes straightforward after the SILGen phase, introduce a raw SIL only - unchecked_ownership instruction for SILGen which will get eliminated during SILGenCleanup.

  %ld = load_borrow %self
  %ex = struct_extract %ld, #Struct.storedProperty
  %unchecked = unchecked_ownership %ex
  end_borrow %ld
  return %unchecked

There are many different markers used today when we cannot resolve ownership right after SILGen. These markers are present until a mandatory pass resolves them. unchecked_ownership can be used as a uniform marker for this purpose in the future.

@meg-gupta
Copy link
Contributor Author

@swift-ci test

@meg-gupta
Copy link
Contributor Author

@swift-ci test

@meg-gupta
Copy link
Contributor Author

@swift-ci test

@meg-gupta meg-gupta marked this pull request as ready for review October 22, 2025 17:48
@meg-gupta
Copy link
Contributor Author

@swift-ci test

@meg-gupta meg-gupta requested a review from atrick October 22, 2025 17:48
@meg-gupta
Copy link
Contributor Author

@swift-ci test

Copy link
Contributor

@eeckstein eeckstein left a comment

Choose a reason for hiding this comment

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

lgtm

@meg-gupta
Copy link
Contributor Author

@swift-ci smoke test macOS platform

This instruction can be used to disable ownership verification on it's result and
will be allowed only in raw SIL.

Sometimes SILGen can produce invalid ownership SSA, that cannot be resolved until
mandatory passes run. We have a few ways to piecewise disable verification.
With unchecked_ownership instruction we can provide a uniform way to disable ownership
verification for a value.
SILGen may produce a borrow accessor result from within a local borrow scope. Such as:

```
  %ld = load_borrow %self
  %fwd = unchecked_ownership %ld
  %ex = struct_extract %fwd, #Struct.storedProperty
  end_borrow %ld
  return %ex
```

This is illegal OSSA, since the return uses a value outside it's borrow scope.

Add a new SILGenCleanup transform, to turn this into valid OSSA:

```
  %ld = load_borrow %self
  %ex = struct_extract %ld, #Struct.storedProperty
  return_borrow %ex from_scopes %ld
```
When return_borrow has both lifetime ending and non-lifetime ending use, consider it lifetime ending.
SILGen inserts mark_unresolved_noncopyable_value at the introducer in some cases and at uses in other cases. This inconsistency can causes insertion of a redundant mark* instruction which crashes the move-only checker.

This change avoids inserting a redundant mark* instruction.
@meg-gupta
Copy link
Contributor Author

@swift-ci smoke test

@meg-gupta meg-gupta enabled auto-merge October 23, 2025 12:19
@meg-gupta meg-gupta merged commit cc0e75c into swiftlang:main Oct 23, 2025
3 checks passed
@atrick
Copy link
Contributor

atrick commented Oct 23, 2025

@meg-gupta My one suggestion on unchecked_ownership_conversion is that everywhere one is created we should have a comment explaining what code is responsible for removing it. Those two pieces of code are closely coupled but in completely different parts of the compiler.

Disable inlining borrow accessors with return_borrow instruction
Disable GenericSpecializer for @guaranteed results

Obvious follow-up tasks?

@meg-gupta
Copy link
Contributor Author

@atrick I'll follow up on comments and disabled cases.

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.

3 participants