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

ARM64: Optimize overflow and divbyzero checks with assertions #98113

Merged
merged 9 commits into from
Feb 9, 2024

Conversation

EgorBo
Copy link
Member

@EgorBo EgorBo commented Feb 7, 2024

Closes #98100

static int Test(int a, int b)
{
    if (b <= 0)  // or other types of similar checks e.g. !=0 or throwing a user exception, etc..
        return 0;

    return a / b;
}

Codegen diff:

; Method Prog:Test(int,int):int (FullOpts)
G_M31864_IG01:
            stp     fp, lr, [sp, #-0x10]!
            mov     fp, sp
G_M31864_IG02:
            cmp     w1, #0
            bgt     G_M31864_IG05
G_M31864_IG03:
            mov     w0, wzr
G_M31864_IG04:
            ldp     fp, lr, [sp], #0x10
            ret     lr
G_M31864_IG05:
-           cmp     w1, #0
-           beq     G_M31864_IG08
-           cmn     w1, #1
-           bne     G_M31864_IG06
-           cmp     w0, #1
-           bvs     G_M31864_IG09
+           sdiv    w0, w0, w1
G_M31864_IG06:
-           sdiv    w0, w0, w1
-G_M31864_IG07:
            ldp     fp, lr, [sp], #0x10
            ret     lr
-G_M31864_IG08:
-           bl      CORINFO_HELP_THROWDIVZERO
-G_M31864_IG09:
-           bl      CORINFO_HELP_OVERFLOW
-           brk_windows #0
-; Total bytes of code: 76
+; Total bytes of code: 40

Should slightly improve certain patterns on x64 as well.

@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 Feb 7, 2024
@ghost ghost assigned EgorBo Feb 7, 2024
@ghost
Copy link

ghost commented Feb 7, 2024

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

Issue Details

Closes #98100

static int Test(int a, int b)
{
    if (b <= 0)  // or other types of similar checks e.g. !=0 or throwing a user exception, etc..
        return 0;

    return a / b;
}

Codegen diff:

; Method Prog:Test(int,int):int (FullOpts)
G_M31864_IG01:
            stp     fp, lr, [sp, #-0x10]!
            mov     fp, sp
G_M31864_IG02:
            cmp     w1, #0
            bgt     G_M31864_IG05
G_M31864_IG03:
            mov     w0, wzr
G_M31864_IG04:
            ldp     fp, lr, [sp], #0x10
            ret     lr
G_M31864_IG05:
-           cmp     w1, #0
-           beq     G_M31864_IG08
-           cmn     w1, #1
-           bne     G_M31864_IG06
-           cmp     w0, #1
-           bvs     G_M31864_IG09
+           sdiv    w0, w0, w1
G_M31864_IG06:
-           sdiv    w0, w0, w1
-G_M31864_IG07:
            ldp     fp, lr, [sp], #0x10
            ret     lr
-G_M31864_IG08:
-           bl      CORINFO_HELP_THROWDIVZERO
-G_M31864_IG09:
-           bl      CORINFO_HELP_OVERFLOW
-           brk_windows #0
-; Total bytes of code: 76
+; Total bytes of code: 40
Author: EgorBo
Assignees: EgorBo
Labels:

area-CodeGen-coreclr

Milestone: -

@EgorBo
Copy link
Member Author

EgorBo commented Feb 7, 2024

On x64 it helps by removing side-effects from divisions, e.g.:

if (b > 0)
{
    var unused = a / b;
}

this can be safely removed now while previously we had to preserve it due to possible exceptions.

@EgorBo
Copy link
Member Author

EgorBo commented Feb 9, 2024

@AndyAyersMS @dotnet/jit-contrib PTAL, a simple change, follow up to #94347 you reviewed. Diffs aren't too big, but I noticed that assertions quite often give up when a tree is wrapped in a cast - so hopefully diffs will improve once I improve that.

Also, it fixes potential problems from existing GTF_DIV_* flags.

src/coreclr/jit/lower.cpp Outdated Show resolved Hide resolved
Copy link
Member

@AndyAyersMS AndyAyersMS left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

We might be able to catch some of these earlier. Feel free to address in a follow-up, if you like.

src/coreclr/jit/assertionprop.cpp Show resolved Hide resolved
@EgorBo EgorBo merged commit 3e9cce6 into dotnet:main Feb 9, 2024
126 of 129 checks passed
@EgorBo EgorBo deleted the optimize-div-mod-arm branch February 9, 2024 14:25
tomeksowi added a commit to tomeksowi/runtime that referenced this pull request Feb 19, 2024
…s::DivideByZeroException is present

Not doing so caused JITting on FullOpts fail on assert(add->acdUsed) in genJumpToThrowHlpBlk_la when the DivByZero check was optimized out as a result of dotnet#98113
jakobbotsch pushed a commit that referenced this pull request Feb 22, 2024
…gned div/mod (#98648)

* Generate check for unsigned divide by zero only when ExceptionSetFlags::DivideByZeroException is present

Not doing so caused JITting on FullOpts fail on assert(add->acdUsed) in genJumpToThrowHlpBlk_la when the DivByZero check was optimized out as a result of #98113

* Change check for GT_DIV or GT_MOD in LSRA to look the same as CodeGen::genCodeForDivMod
@github-actions github-actions bot locked and limited conversation to collaborators Mar 12, 2024
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.

ARM64: redundant overflow and divbyzero checks
3 participants