Skip to content

Conversation

@rjmccall
Copy link
Contributor

@rjmccall rjmccall commented Mar 6, 2020

In order to allow this, I've had to rework the syntax of substituted function types; what was previously spelled <T> in () -> T for <X> is now spelled @substituted <T> () -> T for <X>. I think this is a nice improvement for readability, but it did require me to churn a lot of test cases.

Distinguishing the substitutions has two chief advantages over the existing representation. First, the semantics seem quite a bit clearer at use points; the implicit bit was very subtle and not always obvious how to use. More importantly, it allows the expression of generic function types that must satisfy a particular generic abstraction pattern, which was otherwise impossible to express.

As an example of the latter, consider the following protocol conformance:

protocol P { func foo() }
struct A<T> : P { func foo() {} }

The lowered signature of P.foo is <Self: P> (@in_guaranteed Self) -> (). Without this change, the lowered signature of A.foo's witness would be <T> (@in_guaranteed A<T>) -> (), which does not preserve information about the conformance substitution in any useful way. With this change, the lowered signature of this witness could be <T> @Substituted <Self: P> (@in_guaranteed Self) -> () for <A<T>>, which nicely preserves the exact substitutions which relate the witness to the requirement.

When we adopt this, it will both obviate the need for the special witness-table conformance field in SILFunctionType and make it far simpler for the SILOptimizer to devirtualize witness methods. This patch does not actually take that step, however; it merely makes it possible to do so.

As another piece of unfinished business, while SILFunctionType::substGenericArgs() conceptually ought to simply set the given substitutions as the invocation substitutions, that would disturb a number of places that expect that method to produce an unsubstituted type. This patch only set invocation arguments when the generic type is a substituted type, which we currently never produce in type-lowering.

My plan is to start by producing substituted function types for accessors. Accessors are an important case because the coroutine continuation function is essentially an implicit component of the function type which the current substitution rules simply erase the intended abstraction of. They're also used in narrower ways that should exercise less of the optimizer.

@rjmccall rjmccall requested a review from jckarter March 6, 2020 23:29
@rjmccall
Copy link
Contributor Author

rjmccall commented Mar 6, 2020

@swift-ci Please test

@swift-ci
Copy link
Contributor

swift-ci commented Mar 7, 2020

Build failed
Swift Test Linux Platform
Git Sha - 7792f769cbea2dff68784a07b80d9400a6ba734b

@swift-ci
Copy link
Contributor

swift-ci commented Mar 7, 2020

Build failed
Swift Test OS X Platform
Git Sha - 7792f769cbea2dff68784a07b80d9400a6ba734b

@rjmccall rjmccall force-pushed the invocation-sil-function-subs branch from 7792f76 to d936204 Compare March 7, 2020 03:58
@rjmccall
Copy link
Contributor Author

rjmccall commented Mar 7, 2020

@swift-ci Please test.

@rjmccall rjmccall changed the title [WIP] Invocation/pattern substituted function types. Distinguish invocation and pattern substitutions on SILFunctionType Mar 7, 2020
@swift-ci
Copy link
Contributor

swift-ci commented Mar 7, 2020

Build failed
Swift Test Linux Platform
Git Sha - 7792f769cbea2dff68784a07b80d9400a6ba734b

@varungandhi-apple
Copy link
Contributor

Does docs/SIL.rst need to be updated to explain the new syntax? I don't see the old syntax listed there either, which is perhaps an oversight?

@swift-ci
Copy link
Contributor

swift-ci commented Mar 7, 2020

Build failed
Swift Test OS X Platform
Git Sha - 7792f769cbea2dff68784a07b80d9400a6ba734b

@rjmccall
Copy link
Contributor Author

rjmccall commented Mar 7, 2020

Does docs/SIL.rst need to be updated to explain the new syntax? I don't see the old syntax listed there either, which is perhaps an oversight?

Great point; I'll fix that, either in this PR if it needs a re-run or in a follow-up.

@rjmccall
Copy link
Contributor Author

rjmccall commented Mar 7, 2020

@swift-ci Please test source compatibility

@rjmccall rjmccall force-pushed the invocation-sil-function-subs branch from d936204 to 10df547 Compare March 7, 2020 16:36
@rjmccall
Copy link
Contributor Author

rjmccall commented Mar 7, 2020

Okay, full tests passed. Made some minor changes and re-testing.

@rjmccall
Copy link
Contributor Author

rjmccall commented Mar 7, 2020

@swift-ci Please smoke test and merge.

1 similar comment
@rjmccall
Copy link
Contributor Author

rjmccall commented Mar 7, 2020

@swift-ci Please smoke test and merge.

In order to allow this, I've had to rework the syntax of substituted function types; what was previously spelled `<T> in () -> T for <X>` is now spelled `@substituted <T> () -> T for <X>`.  I think this is a nice improvement for readability, but it did require me to churn a lot of test cases.

Distinguishing the substitutions has two chief advantages over the existing representation.  First, the semantics seem quite a bit clearer at use points; the `implicit` bit was very subtle and not always obvious how to use.  More importantly, it allows the expression of generic function types that must satisfy a particular generic abstraction pattern, which was otherwise impossible to express.

As an example of the latter, consider the following protocol conformance:

```
protocol P { func foo() }
struct A<T> : P { func foo() {} }
```

The lowered signature of `P.foo` is `<Self: P> (@in_guaranteed Self) -> ()`.  Without this change, the lowered signature of `A.foo`'s witness would be `<T> (@in_guaranteed A<T>) -> ()`, which does not preserve information about the conformance substitution in any useful way.  With this change, the lowered signature of this witness could be `<T> @Substituted <Self: P> (@in_guaranteed Self) -> () for <A<T>>`, which nicely preserves the exact substitutions which relate the witness to the requirement.

When we adopt this, it will both obviate the need for the special witness-table conformance field in SILFunctionType and make it far simpler for the SILOptimizer to devirtualize witness methods.  This patch does not actually take that step, however; it merely makes it possible to do so.

As another piece of unfinished business, while `SILFunctionType::substGenericArgs()` conceptually ought to simply set the given substitutions as the invocation substitutions, that would disturb a number of places that expect that method to produce an unsubstituted type.  This patch only set invocation arguments when the generic type is a substituted type, which we currently never produce in type-lowering.

My plan is to start by producing substituted function types for accessors.  Accessors are an important case because the coroutine continuation function is essentially an implicit component of the function type which the current substitution rules simply erase the intended abstraction of.  They're also used in narrower ways that should exercise less of the optimizer.
@rjmccall rjmccall force-pushed the invocation-sil-function-subs branch from 10df547 to ceff414 Compare March 7, 2020 21:26
@rjmccall
Copy link
Contributor Author

rjmccall commented Mar 7, 2020

@swift-ci Please smoke test and merge.

1 similar comment
@rjmccall
Copy link
Contributor Author

rjmccall commented Mar 7, 2020

@swift-ci Please smoke test and merge.

@rjmccall
Copy link
Contributor Author

rjmccall commented Mar 7, 2020

@swift-ci Please clean smoke test OS X

@rjmccall
Copy link
Contributor Author

rjmccall commented Mar 7, 2020

@swift-ci Please clean smoke test OS X platform

@rjmccall rjmccall merged commit 1f62fe5 into swiftlang:master Mar 7, 2020
@rjmccall rjmccall deleted the invocation-sil-function-subs branch March 7, 2020 23:45
@jckarter
Copy link
Contributor

jckarter commented Mar 9, 2020

Thanks @rjmccall! In addition to SIL.rst, it might be a good idea to mention the new syntax and generic signature behavior in the Swift Forums development thread: https://forums.swift.org/t/improving-the-representation-of-polymorphic-interfaces-in-sil-with-substituted-function-types/29711

@rjmccall
Copy link
Contributor Author

Thanks @rjmccall! In addition to SIL.rst, it might be a good idea to mention the new syntax and generic signature behavior in the Swift Forums development thread: https://forums.swift.org/t/improving-the-representation-of-polymorphic-interfaces-in-sil-with-substituted-function-types/29711

Great idea! I've posted in that thread.

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.

4 participants