-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
Try to loosen cfunction optimization type check #21467
Conversation
can whatever was wrong with the previous version be tested for? |
Nothing was wrong. It's only a performance issue. |
74ba371
to
1db0284
Compare
src/ccall.cpp
Outdated
fargt = jl_tparam0(fargt); | ||
if (jl_has_free_typevars(fargt)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
|| !jl_is_tuple_type(fargt)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
1db0284
to
785ee2c
Compare
785ee2c
to
3660535
Compare
Any other comments? |
worth running nanosoldier? |
Kind of doubt it has anything that uses cfunction but sure.... @nanosoldier |
Your benchmark job has completed - possible performance regressions were detected. A full report can be found here. cc @jrevels |
Not sure if this is precise now but the old one seems to be too strict since it doesn't catch passing by ref or non-leaf argument types.