-
-
Notifications
You must be signed in to change notification settings - Fork 30.4k
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
gh-120619: Strength reduce function guards, support 2-operand instruction forms #124846
gh-120619: Strength reduce function guards, support 2-operand instruction forms #124846
Conversation
@markshannon can you please bengmchmark this? |
No significant performance impact, 0.3% faster on average. |
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.
Looks generally sound.
One assert and comment seem incorrect.
Maybe rename operand
to operand0
if we are adding operand1
When you're done making the requested changes, leave the comment: |
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.
There's a test that can be removed, otherwise LGTM.
Looks good to me. @brandtbucher can you approve if you are happy with the JIT changes? |
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.
JIT changes look good, just a couple out-of-date comments:
Co-authored-by: Brandt Bucher <[email protected]>
_CHECK_FUNCTION_EXACT_ARGS
if we can statically determine the number of arguments at optimization time._CHECK_FUNCTION_VERSION
to not need to read the stack operands. To do this, we need a 2-operand form of uops. This PR thus adds support for that.Altogether, these prepare it for partial evaluation in the future, as after removing these guards, this means the inputs do not need to be materialized.