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

Convert DIV/MOD to UDIV/UMOD using assertions #94347

Merged
merged 9 commits into from
Nov 6, 2023

Conversation

EgorBo
Copy link
Member

@EgorBo EgorBo commented Nov 3, 2023

Closes #94218

static int Test(int i)
{
    if (i < 4) return 0;
    return i / 4;
}
; Method Benchmarks:Test(int):int (FullOpts)
       cmp      ecx, 4
       jge      SHORT G_M33641_IG05
       xor      eax, eax
       ret      
G_M33641_IG05:
       mov      eax, ecx
-      sar      eax, 31
-      and      eax, 3
-      add      eax, ecx
-      sar      eax, 2
+      shr      eax, 2
       ret      
-; Total bytes of code: 22
+; Total bytes of code: 14

@ghost ghost assigned EgorBo Nov 3, 2023
@dotnet-issue-labeler dotnet-issue-labeler bot added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Nov 3, 2023
@ghost
Copy link

ghost commented Nov 3, 2023

Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch
See info in area-owners.md if you want to be subscribed.

Issue Details

Closes #94218

Author: EgorBo
Assignees: EgorBo
Labels:

area-CodeGen-coreclr

Milestone: -

@EgorBo EgorBo marked this pull request as ready for review November 4, 2023 13:50
@EgorBo
Copy link
Member Author

EgorBo commented Nov 4, 2023

@AndyAyersMS @dotnet/jit-contrib PTAL, diffs are not too big, I think I saw in some places in BCL we explicitly add (uint) cast for better CQ so this PR should allow to use that hack less often. Currently, it doesn't support long because we don't create assertions for those (I'll look separately if I can cheaply enable them) as these assertions mainly focus on bound checks.

Also, I remove GTF_DIV_BY_CNS_OPT as it doesn't seem to have any value to occupy a flag.

src/coreclr/jit/assertionprop.cpp Outdated Show resolved Hide resolved
src/coreclr/jit/assertionprop.cpp Outdated Show resolved Hide resolved
src/coreclr/jit/assertionprop.cpp Outdated Show resolved Hide resolved
@EgorBo EgorBo merged commit 6066de1 into dotnet:main Nov 6, 2023
127 of 129 checks passed
@EgorBo
Copy link
Member Author

EgorBo commented Nov 6, 2023

Failure is #86019

@EgorBo EgorBo deleted the use-assertions-div branch November 6, 2023 13:19
@ghost ghost locked as resolved and limited conversation to collaborators Dec 6, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Signed division performed even when values are provably non-negative
4 participants