JIT: Add morphing to LE_UN/GT_UN(expr, uint.MaxValue) (dotnet#76525) #113037
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ghost
added
the
area-CodeGen-coreclr
dotnet-policy-service
bot
added
the
community-contribution
|
Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch |
|
@dotnet-policy-service agree company="Microsoft" |
EgorBo
reviewed
Mar 2, 2025
|
Seems like CI failed with |
EgorBo
reviewed
Mar 2, 2025
That's probably because this PR isn't handling the things pointed out here: #76525 (comment) |
Thank you for pointing it out. Will check the issue comment and fix. |
shunkino
force-pushed
the
jit-uint-maxvalue-compare-76525
branch
2 times, most recently
from
4fe1edc to
907e97c
Compare
shunkino
commented
Mar 26, 2025
jakobbotsch
reviewed
Mar 26, 2025
|
There are some cases in the diffs where the optimization is not kicking in: Can you check why? |
|
I just noticed that the above was a tier 0 method. It looks like |
* Add morphing for comparison between uint.MaxValue and a ulong value. * Remove assertions, which no longer is relevant. * Add a check to lsra to only use BMI2 if zero flag will not be set. * Add a profitability check to optimize EQ/NE(op, 0) only if op has constant operand.
shunkino
force-pushed
the
jit-uint-maxvalue-compare-76525
branch
from
907e97c to
440adf9
Compare
shunkino
commented
Apr 1, 2025
|
/azp run Fuzzlyn |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
Hi @jakobbotsch, I tried to review the Fuzzlyn results, but there are many errors that I’m having trouble interpreting. Sorry to bother you, but could you help me understand what they mean or guide me on how to proceed? Thanks in advance! |
|
I think the Fuzzlyn found issues are unrelated to this PR. I need to take another look, but I don't think anything more is needed from your side. |
jakobbotsch
reviewed
May 13, 2025
|
@jakobbotsch |
|
Try moving !tree->gtSetFlags() to line 4866... |
This reverts commit 00acd28.
|
@jakobbotsch Thanks for checking. Reverted the change to the one I commented here. |
|
/azp run runtime-coreclr superpmi-replay |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
The failure is #115070, not sure why it is not being picked up automatically. |
jakobbotsch
approved these changes
May 19, 2025
|
/ba-g Failure is #115070 |
|
FYI jit-format failed here. I will fix as part of #115731. |
|
Weird, looks like it failed 7 minutes after I merged this. Not sure why the failure wasn't there before that. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Add morphing for comparison between uint.MaxValue and a ulong value.
Contribute to: #76525