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

Mark typeintersect pure for Julia 1.9 #1156

Merged
merged 7 commits into from
Apr 27, 2023

Conversation

vchuravy
Copy link
Contributor

Fixes the concrete issue in #1155.

Julia 1.9 favors concrete-eval over constant propagation.
Furthermore the effect system was powered up and many @pure annotation were removed.

With --check-bounds=no concrete-eval is disabled
and we rely entirely on const-prop, which seems to struggle whithout the @pure annotation.

@aviatesk would it be feasible to consider the effects for directing const-prop? We have two situations
where the reliance on concrete-eval has shown itself to be problematic. GPU code with custom method-tables,
and --check-bounds=no.

In Julia 1.10 @pure is deprecated and the testcase in #1155
fails again.

@vchuravy vchuravy changed the title Mark typeintersect pure for Julia 1.9+ Mark typeintersect pure for Julia 1.9 Apr 24, 2023
@aviatesk
Copy link

@aviatesk would it be feasible to consider the effects for directing const-prop?

Yes, we can do something like JuliaLang/julia#46471 (or even set up a separate system to track const-prop' profitability).

Having said that, typeintersect is not inferrable by const-prop', since the main logic is implemented by C.

@vchuravy
Copy link
Contributor Author

Having said that, typeintersect is not inferrable by const-prop', since the main logic is implemented by C.

Right it used to have an @pure annotation for that reason.

Copy link
Contributor

@ranocha ranocha left a comment

Choose a reason for hiding this comment

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

Thanks a lot for the fix, @vchuravy! Would it make sense to add some tests for this? This would basically need to call Julia again but with the appropriate command line flags since testing uses bounds checking enabled by default.

@ranocha
Copy link
Contributor

ranocha commented Apr 26, 2023

@vchuravy I made a PR adding some tests to your branch. Once it is merged, this PR looks good to me. Thanks a lot for investigating this issue and creating a workaround so quickly!

add tests to the PR Mark typeintersect pure for Julia 1.9
Copy link
Contributor

@ranocha ranocha left a comment

Choose a reason for hiding this comment

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

This looks good to me and is a great fix for some performance problems we encounter with Julia v1.9 as downstream users of StaticArrays.jl.

Copy link
Collaborator

@mateuszbaran mateuszbaran left a comment

Choose a reason for hiding this comment

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

LGTM. Could you just add Julia 1.9-dev to CI testing and bump patch version?

@mateuszbaran mateuszbaran merged commit ed92217 into JuliaArrays:master Apr 27, 2023
@sloede
Copy link

sloede commented Apr 27, 2023

Great, thanks a lot to everyone involved for creating a fix so fast. Once a new version of StaticArrays.jl has been released, I will run some performance benchmarks on Julia v1.9 with --check-bounds=no 🐎

@vchuravy vchuravy deleted the vc/pure_typeintersect branch April 27, 2023 19:45
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.

5 participants