-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
Fold "X >= 0 && X < NN" to "X u< NN" #93834
Conversation
Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch Issue DetailsFold e.g.
static void Test(Span<int> span, int i)
{
if (i >= 0 && i < span.Length)
Console.WriteLine("In range!");
} ; Method InitblkPerformanceIssue.Program:Test(System.Span`1[int],int) (FullOpts)
sub rsp, 40
- test edx, edx
- jl SHORT G_M55087_IG04
cmp edx, dword ptr [rcx+0x08]
- jge SHORT G_M55087_IG04
+ jae SHORT G_M55087_IG04
mov rcx, 0x1AC80209320 ; 'In range!'
call [System.Console:WriteLine(System.String)]
G_M55087_IG04:
nop
add rsp, 40
ret
-; Total bytes of code: 35
+; Total bytes of code: 31
|
Would still prefer we sort these out earlier, but also like having some "defense in depth."
Once take this change, can we undo some of these? |
@EgorBo just a few notes on the test as I looked through it due to #94146. Some are just ideas to make test authoring easier in the future and might not be worth retroactive changes
|
Please cc @dotnet/jit-contrib for JIT PRs. |
It wasn't merged by me, I planned to ask for review once I am back from vacation 🤷♂️ It's unfortunate that @markples thanks, will apply the suggestions as part of some other PR |
Sorry, i was going through the "approved PRs" list and merged this one.
Do we know why this happened and if there is a fix needed? |
#94146 (comment) it's not clear yet |
* Fold "X >= 0 && X < NN" to "X u< NN" * Relax to GTF_SIDE_EFFECT
Fold e.g.
X >= 0 && X < arr.Length
to(uint)X < arr.Length
. Not too many diffs because:uint
hack (I call it a hack because IMO it's not how most developers check ranges. Also, it's not "use checked context" friendly) everywhere in BCL