Skip to content
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

codegen: improve our argument annotations #35463

Merged
merged 1 commit into from
Apr 18, 2020
Merged

codegen: improve our argument annotations #35463

merged 1 commit into from
Apr 18, 2020

Conversation

vtjnash
Copy link
Sponsor Member

@vtjnash vtjnash commented Apr 13, 2020

Reflect that immutable struct parameters are readonly by the time LLVM
can see them, even passed inside boxes.

@vchuravy

This comment has been minimized.

@JeffBezanson
Copy link
Sponsor Member

The interesting thing about this is that these annotations should be correct and useful, but in fact they somehow block the vectorizer. This simulates what's happening in the performance regressions in #34126.

Reflect that immutable struct parameters are readonly by the time LLVM
can see them, even passed inside boxes.

LLVM seems to require an explicit additional LICM call, to avoid
regressions from this (observing the scheduling of fewer LICM calls).
@vtjnash
Copy link
Sponsor Member Author

vtjnash commented Apr 14, 2020

@nanosoldier runbenchmarks(ALL, vs=":master")

@nanosoldier
Copy link
Collaborator

Your benchmark job has completed - possible performance regressions were detected. A full report can be found here. cc @ararslan

@MasonProtter

This comment has been minimized.

@vtjnash
Copy link
Sponsor Member Author

vtjnash commented Apr 15, 2020

Fun to see it improved ["misc", "afoldl", "Float64"]! Should be good to go, unless someone has thoughts about a better way to do this.

@JeffBezanson
Copy link
Sponsor Member

The extra LICM pass doesn't seem to have much latency impact from what I can see so far, so let's go with this for now.

@JeffBezanson JeffBezanson merged commit c3fc367 into master Apr 18, 2020
@JeffBezanson JeffBezanson deleted the jn/arg_const branch April 18, 2020 20:13
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.

None yet

5 participants