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

JIT is missing a common case to track the IsNeverNegative assertion #102088

Closed
tannergooding opened this issue May 10, 2024 · 1 comment · Fixed by #102089
Closed

JIT is missing a common case to track the IsNeverNegative assertion #102088

tannergooding opened this issue May 10, 2024 · 1 comment · Fixed by #102089
Assignees
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

@tannergooding
Copy link
Member

Given the following code:

public static int M1(int y)
{
    if (y < 0) { return y; }
    return y % 8;
}

public static uint M2(uint y)
{
    if ((int)y < 0) { return y; }
    return y % 8;
}

We currently generate the same assembly:

mov      eax, ecx
and      eax, 7
test     ecx, ecx
cmovl    eax, ecx

This is due to the if (y < 0) { return y; } check in M1 allowing the JIT to determine that y is never negative when it comes to the y % 8 expression.


However, in the case of the the following:

public static int M3(Span<int> x, int y)
{
    return x[y] + (y % 8);
}

We instead generate the following for the y % 8:

mov      eax, ecx
sar      eax, 31
and      eax, 7
add      eax, ecx
and      eax, -8
sub      ecx, eax
mov      eax, ecx

This is unexpected since x[y] for both Span<T> and T[] is known to require a positive index and to throw if the index is not positive or greater than Length (i.e the bounds check fails). As such, it should be possible for the JIT to assert that y is never negative after it is successfully used as an index into a span or array.

While the code example given above is minimal and not necessarily representative of real world code, there are many places where such an index is used for latter indexing or computation and thus tracking the fact can lead to other downstream codegen improvements (whether that be simplified bounds checks elsewhere or optimizations such as is done for %).

@tannergooding tannergooding added tenet-performance Performance related issue area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI labels May 10, 2024
@dotnet-policy-service dotnet-policy-service bot added the untriaged New issue has not been triaged by the area owner label May 10, 2024
@EgorBo EgorBo self-assigned this May 10, 2024
@EgorBo EgorBo removed the untriaged New issue has not been triaged by the area owner label May 10, 2024
@EgorBo EgorBo added this to the 9.0.0 milestone May 10, 2024
Copy link
Contributor

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

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 tenet-performance Performance related issue
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants