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

Unnecessary overflow check with checked on 64-bit ryujit #111567

Open
hamarb123 opened this issue Jan 18, 2025 · 3 comments
Open

Unnecessary overflow check with checked on 64-bit ryujit #111567

hamarb123 opened this issue Jan 18, 2025 · 3 comments
Labels
area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI tenet-performance Performance related issue
Milestone

Comments

@hamarb123
Copy link
Contributor

Description

Elidable checked is not elided.

/cc @EgorBo who asked me to open an issue for it

Configuration

64-bit on sharplab.com

Regression?

Probably not.

Data

public class C {
    public nuint M(uint a) {
        return checked((nuint)a * 8);
    }
}

gives

C.M(UInt32)
    L0000: sub rsp, 0x28
    L0004: mov eax, edx
    L0006: mov ecx, 8
    L000b: mul rcx
    L000e: jb short L0015
    L0010: add rsp, 0x28
    L0014: ret
    L0015: call 0x0000027c46350090
    L001a: int3

Sharplab link

Analysis

The checked can be elided on 64-bit, since a fits within 32 bits & a * 8 fits within 35 bits, which means no overflow can occur with 64-bits of storage.

Seemingly doesn't reproduce on 32-bit when nuint is replaced with ulong (but still does on 64-bit)? Sharplab link

@hamarb123 hamarb123 added the tenet-performance Performance related issue label Jan 18, 2025
@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 Jan 18, 2025
@dotnet-policy-service dotnet-policy-service bot added the untriaged New issue has not been triaged by the area owner label Jan 18, 2025
Copy link
Contributor

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

@EgorBo EgorBo added this to the Future milestone Jan 18, 2025
@EgorBo EgorBo removed the untriaged New issue has not been triaged by the area owner label Jan 18, 2025
@hamarb123 hamarb123 changed the title Unnecessary overflow check with checked Unnecessary overflow check with checked on 64-bit ryujit Jan 18, 2025
@xtqqczze
Copy link
Contributor

xtqqczze commented Jan 18, 2025

Elidable checked is not elided.

The code generation issue is actually worse, using checked forces use of mul instead of shl:

C.Unchecked(UInt32)
    L0000: mov eax, edx
    L0002: shl rax, 3
    L0006: ret

C.Checked(UInt32)
    L0000: sub rsp, 0x28
    L0004: mov eax, edx
    L0006: mov ecx, 8
    L000b: mul rcx
    L000e: jb short L0015
    L0010: add rsp, 0x28
    L0014: ret
    L0015: call 0x0000027c464a00b0
    L001a: int3

SharpLab

@huoyaoyuan
Copy link
Member

Also #105333

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI tenet-performance Performance related issue
Projects
None yet
Development

No branches or pull requests

4 participants