Skip to content

Sema: Clean up handling of protocol operators with concrete operands #28788

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

slavapestov
Copy link
Contributor

@slavapestov slavapestov commented Dec 13, 2019

More peeling off refactorings from #28698.

In this case we would "devirtualize" the protocol requirement call
by building the AST to model a direct reference to the witness.

Previously this was done by recursively calling typeCheckExpression(),
but the only thing this did was recover the correct substitutions
for the call.

Instead, we can just build the right SubstitutionMap directly.
Unfortunately, while we serialize enough information in the AST
to devirtualize calls at the SIL level, we do not for AST Exprs.

This is because SIL devirtualization builds a reference to the
witness thunk signature, which is an intermediate step between
the protocol requirement and the witness. I get around this by
deriving the substitutions from walking in parallel over the
interface type of the witness, together with the inferred type
of the call expression.

@slavapestov slavapestov requested a review from xedin December 13, 2019 23:16
@slavapestov
Copy link
Contributor Author

@DougGregor @xedin This eliminates the second-to-last typeCheckExpression() call from inside CSApply. The last one is in the handling of EditorPlaceholderExpr.

@slavapestov slavapestov force-pushed the csapply-protocol-operator-cleanup branch from 15beef8 to d13f603 Compare December 14, 2019 04:25
In this case we would "devirtualize" the protocol requirement call
by building the AST to model a direct reference to the witness.

Previously this was done by recursively calling typeCheckExpression(),
but the only thing this did was recover the correct substitutions
for the call.

Instead, we can just build the right SubstitutionMap directly.
Unfortunately, while we serialize enough information in the AST
to devirtualize calls at the SIL level, we do not for AST Exprs.

This is because SIL devirtualization builds a reference to the
witness thunk signature, which is an intermediate step between
the protocol requirement and the witness. I get around this by
deriving the substitutions from walking in parallel over the
interface type of the witness, together with the inferred type
of the call expression.
@slavapestov slavapestov force-pushed the csapply-protocol-operator-cleanup branch from d13f603 to 594044a Compare December 14, 2019 06:02
@slavapestov
Copy link
Contributor Author

@swift-ci Please smoke test

@slavapestov
Copy link
Contributor Author

@swift-ci Please test source compatibility

@slavapestov
Copy link
Contributor Author

@swift-ci Please test source compatibility Release

@slavapestov
Copy link
Contributor Author

@swift-ci Please benchmark

@slavapestov
Copy link
Contributor Author

@swift-ci Please test source compatibility Release

@swift-ci
Copy link
Contributor

Performance: -O

Code size: -O

Performance: -Osize

Regression OLD NEW DELTA RATIO
FlattenListLoop 2654 3136 +18.2% 0.85x (?)

Code size: -Osize

Performance: -Onone

Code size: -swiftlibs

How to read the data The tables contain differences in performance which are larger than 8% and differences in code size which are larger than 1%.

If you see any unexpected regressions, you should consider fixing the
regressions before you merge the PR.

Noise: Sometimes the performance results (not code size!) contain false
alarms. Unexpected regressions which are marked with '(?)' are probably noise.
If you see regressions which you cannot explain you can try to run the
benchmarks again. If regressions still show up, please consult with the
performance team (@eeckstein).

Hardware Overview
  Model Name: Mac mini
  Model Identifier: Macmini8,1
  Processor Name: Intel Core i7
  Processor Speed: 3.2 GHz
  Number of Processors: 1
  Total Number of Cores: 6
  L2 Cache (per Core): 256 KB
  L3 Cache: 12 MB
  Memory: 64 GB

@slavapestov slavapestov merged commit 0a89228 into swiftlang:master Dec 14, 2019
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