Skip to content

[moveOnly] Add a skeleton _move function #39943

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged

Conversation

gottesmm
Copy link
Contributor

@gottesmm gottesmm commented Oct 28, 2021

[moveOnly] Implement a semi-generic _move function that can be used on non-generic, non-existential values.

This patch introduces a new stdlib function called _move:

  @_alwaysEmitIntoClient
  @_transparent
  @_semantics("lifetimemanagement.move")
  public func _move<T>(_ value: __owned T) -> T {
  #if $BuiltinMove
    Builtin.move(value)
  #else
    value
  #endif
  }

It is a first attempt at creating a "move" function for Swift, albeit a skleton
one since we do not yet perform the "no use after move" analysis. But this at
leasts gets the skeleton into place so we can built the analysis on top of it
and churn tree in a manageable way. Thus in its current incarnation, all it does
is take in an __owned +1 parameter and returns it after moving it through
Builtin.move.

Given that we want to use an OSSA based analysis for our "no use after move"
analysis and we do not have opaque values yet, we can not supporting moving
generic values since they are address only. This has stymied us in the past from
creating this function. With the implementation in this PR via a bit of
cleverness, we are now able to support this as a generic function over all
concrete types by being a little clever.

The trick is that when we transparent inline _move (to get the builtin), we
perform one level of specialization causing the inlined Builtin.move to be of a
loadable type. If after transparent inlining, we inline builtin "move" into a
context where it is still address only, we emit a diagnostic telling the user
that they applied move to a generic or existential and that this is not yet
supported.

The reason why we are taking this approach is that we wish to use this to
implement a new (as yet unwritten) diagnostic pass that verifies that _move
(even for non-trivial copyable values) ends the lifetime of the value. This will
ensure that one can write the following code to reliably end the lifetime of a
let binding in Swift:

  let x = Klass()
  let _ = _move(x)
  // hypotheticalUse(x)

Without the diagnostic pass, if one were to write another hypothetical use of x
after the _move, the compiler would copy x to at least hypotheticalUse(x)
meaning the lifetime of x would not end at the _move, =><=.

So to implement this diagnostic pass, we want to use the OSSA infrastructure and
that only works on objects! So how do we square this circle: by taking advantage
of the mandatory SIL optimzier pipeline! Specifically we take advantage of the
following:

  1. Mandatory Inlining and Predictable Dead Allocation Elimination run before any
    of the move only diagnostic passes that we run.

  2. Mandatory Inlining is able to specialize a callee a single level when it
    inlines code. One can take advantage of this to even at -Onone to
    monomorphosize code.

and then note that _move is such a simple function that predictable dead
allocation elimination is able to without issue eliminate the extra alloc_stack
that appear in the caller after inlining without issue. So we (as the tests
show) get SIL that for concrete types looks exactly like we just had run a
move_value for that specific type as an object since we promote away the
stores/loads in favor of object operations when we eliminate the allocation.

In order to prevent any issue with this being used in a context where multiple
specializations may occur, I made the inliner emit a diagnostic if it inlines
_move into a function that applies it to an address only value. The diagnostic
is emitted at the source location where the function call occurs so it is easy
to find, e.x.:

func addressOnlyMove<T>(t: T) -> T {
    _move(t) // expected-error {{move() used on a generic or existential value}}
}

moveonly_builtin_generic_failure.swift:12:5: error: move() used on a generic or existential value
    _move(t)
    ^

To eliminate any potential ABI impact, if someone calls _move in a way that
causes it to be used in a context where the transparent inliner will not inline
it, I taught IRGen that Builtin.move is equivalent to a take from src -> dst and
marked _move as always emit into client (AEIC). I also took advantage of the
feature flag I added in the previous commit in order to prevent any cond_fails
from exposing Builtin.move in the stdlib. If one does not pass in the flag
-enable-experimental-move-only then the function just returns the value without
calling Builtin.move, so we are safe.

rdar://83957028

…ond_fail errors.

We use it in a subsequent commit in this PR to guard BuiltinMove.
…rol usage of move only features.

These include _move and @_noImplicitCopy. I still need to wire up the parsing of
those behind this feature.

The reason that I am adding this now is that I am going to now need to make some
changes behind a feature flag and I have not yet needed to add one. The specific
reason I needed to add one here is to ensure that I properly guard inside _move
the call to Builtin.move so as to prevent a "cond_fail" incident.

P.S.: This work depends on experimental lexical lifetimes being enabled as well,
so I did that at the same time in this PR.
…n non-generic, non-existential values.

This patch introduces a new stdlib function called _move:

```Swift
  @_alwaysEmitIntoClient
  @_transparent
  @_semantics("lifetimemanagement.move")
  public func _move<T>(_ value: __owned T) -> T {
  #if $ExperimentalMoveOnly
    Builtin.move(value)
  #else
    value
  #endif
  }
```

It is a first attempt at creating a "move" function for Swift, albeit a skleton
one since we do not yet perform the "no use after move" analysis. But this at
leasts gets the skeleton into place so we can built the analysis on top of it
and churn tree in a manageable way. Thus in its current incarnation, all it does
is take in an __owned +1 parameter and returns it after moving it through
Builtin.move.

Given that we want to use an OSSA based analysis for our "no use after move"
analysis and we do not have opaque values yet, we can not supporting moving
generic values since they are address only. This has stymied us in the past from
creating this function. With the implementation in this PR via a bit of
cleverness, we are now able to support this as a generic function over all
concrete types by being a little clever.

The trick is that when we transparent inline _move (to get the builtin), we
perform one level of specialization causing the inlined Builtin.move to be of a
loadable type. If after transparent inlining, we inline builtin "move" into a
context where it is still address only, we emit a diagnostic telling the user
that they applied move to a generic or existential and that this is not yet
supported.

The reason why we are taking this approach is that we wish to use this to
implement a new (as yet unwritten) diagnostic pass that verifies that _move
(even for non-trivial copyable values) ends the lifetime of the value. This will
ensure that one can write the following code to reliably end the lifetime of a
let binding in Swift:

```Swift
  let x = Klass()
  let _ = _move(x)
  // hypotheticalUse(x)
```

Without the diagnostic pass, if one were to write another hypothetical use of x
after the _move, the compiler would copy x to at least hypotheticalUse(x)
meaning the lifetime of x would not end at the _move, =><=.

So to implement this diagnostic pass, we want to use the OSSA infrastructure and
that only works on objects! So how do we square this circle: by taking advantage
of the mandatory SIL optimzier pipeline! Specifically we take advantage of the
following:

1. Mandatory Inlining and Predictable Dead Allocation Elimination run before any
   of the move only diagnostic passes that we run.

2. Mandatory Inlining is able to specialize a callee a single level when it
   inlines code. One can take advantage of this to even at -Onone to
   monomorphosize code.

and then note that _move is such a simple function that predictable dead
allocation elimination is able to without issue eliminate the extra alloc_stack
that appear in the caller after inlining without issue. So we (as the tests
show) get SIL that for concrete types looks exactly like we just had run a
move_value for that specific type as an object since we promote away the
stores/loads in favor of object operations when we eliminate the allocation.

In order to prevent any issue with this being used in a context where multiple
specializations may occur, I made the inliner emit a diagnostic if it inlines
_move into a function that applies it to an address only value. The diagnostic
is emitted at the source location where the function call occurs so it is easy
to find, e.x.:

```
func addressOnlyMove<T>(t: T) -> T {
    _move(t) // expected-error {{move() used on a generic or existential value}}
}

moveonly_builtin_generic_failure.swift:12:5: error: move() used on a generic or existential value
    _move(t)
    ^
```

To eliminate any potential ABI impact, if someone calls _move in a way that
causes it to be used in a context where the transparent inliner will not inline
it, I taught IRGen that Builtin.move is equivalent to a take from src -> dst and
marked _move as always emit into client (AEIC). I also took advantage of the
feature flag I added in the previous commit in order to prevent any cond_fails
from exposing Builtin.move in the stdlib. If one does not pass in the flag
-enable-experimental-move-only then the function just returns the value without
calling Builtin.move, so we are safe.

rdar://83957028
@gottesmm gottesmm force-pushed the pr-65ca66f5402c00e0f0fa0608409ef1c732d1fd2b branch from cf249c5 to 1147897 Compare October 28, 2021 02:49
@gottesmm
Copy link
Contributor Author

@swift-ci smoke test

@gottesmm gottesmm changed the title [moveOnly] Add a skeleton _move function behind the -enable-experimental-move-only flag [moveOnly] Add a skeleton _move function Oct 28, 2021
@gottesmm gottesmm merged commit 60f3d61 into swiftlang:main Oct 28, 2021
@gottesmm gottesmm deleted the pr-65ca66f5402c00e0f0fa0608409ef1c732d1fd2b branch October 28, 2021 09:23
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.

1 participant