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

[API Proposal]: An official way to determine whether the given value is JIT-time constant or not #98153

Closed
MineCake147E opened this issue Feb 8, 2024 · 11 comments
Labels
api-suggestion Early API idea and discussion, it is NOT ready for implementation area-System.Runtime.CompilerServices blocked Issue/PR is blocked on something - see comments

Comments

@MineCake147E
Copy link
Contributor

Background and motivation

[MethodImpl(MethodImplOptions.AggressiveInlining | MethodImplOptions.AggressiveOptimization)]
public static bool IsConstant(int value)
    => Vector128.CreateScalarUnsafe(value - ~value).GetElement(1) != 0;
    L0000: vzeroupper
    L0003: mov eax, ecx
    L0005: not eax
    L0007: sub ecx, eax
    L0009: vmovd xmm0, ecx
    L000d: vpextrd eax, xmm0, 1
    L0013: test eax, eax
    L0015: setne al
    L0018: movzx eax, al
    L001b: ret

The code above currently allows me to guess whether the value is a JIT-time constant in x86-64 environments.
The Vector128.CreateScalarUnsafe(value) usually does the same thing with Vector128.CreateScalar(value) (which is vmovd xmm*, r*d) which clears all upper elements with 0, but it does Vector128.Create(value) which broadcasts the value, when the value is a constant.
The value - ~value is guaranteed to be nonzero, as LLVM says, and it obviously differs by value.
By checking whether the second element is nonzero or not, it returns true when the value is a constant, false otherwise.
Inlining the IsConstant(constant) simply boils down to mov eax, 1 as shown here.

Such behavior is really useful when I deal with some SIMD optimization, as some instructions only take constant value.
It also helps me to replicate something like what System.Buffer.Memmove does for small constant lengths, without modifying the CoreCLR.
But it's an undefined behavior, and I'm not sure if it works with ARM-based processors, so I propose the APIs below for official support.

API Proposal

namespace System.Runtime.CompilerServices;

public static class Unsafe
{
    public static bool IsConstant<T>(T value) where T : unmanaged;
}

API Usage

// The method takes `[ConstantExpected] int index` and `Vector512<ushort> zmm2` as parameter
if (Unsafe.IsConstant(index))
{
    // some processing that needs the index to be a constant value
    return zmm2.GetElement(index);
}
// some processing that don't need the index to be a constant value
return Avx512BW.PermuteVar32x16(zmm2, Vector512.CreateScalarUnsafe((ushort)index)).GetElement(0);

Alternative Designs

  • There may be a better place for IsConstant to be.
  • There may be a better name for IsConstant.

Risks

None

@MineCake147E MineCake147E added the api-suggestion Early API idea and discussion, it is NOT ready for implementation label Feb 8, 2024
@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 8, 2024
@ghost ghost added the untriaged New issue has not been triaged by the area owner label Feb 8, 2024
@ghost
Copy link

ghost commented Feb 8, 2024

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

Issue Details

Background and motivation

[MethodImpl(MethodImplOptions.AggressiveInlining | MethodImplOptions.AggressiveOptimization)]
public static bool IsConstant(int value)
    => Vector128.CreateScalarUnsafe(value - ~value).GetElement(1) != 0;
    L0000: vzeroupper
    L0003: mov eax, ecx
    L0005: not eax
    L0007: sub ecx, eax
    L0009: vmovd xmm0, ecx
    L000d: vpextrd eax, xmm0, 1
    L0013: test eax, eax
    L0015: setne al
    L0018: movzx eax, al
    L001b: ret

The code above currently allows me to guess whether the value is a JIT-time constant in x86-64 environments.
The Vector128.CreateScalarUnsafe(value) usually does the same thing with Vector128.CreateScalar(value) (which is vmovd xmm*, r*d) which clears all upper elements with 0, but it does Vector128.Create(value) which broadcasts the value, when the value is a constant.
The value - ~value is guaranteed to be nonzero, as LLVM says, and it obviously differs by value.
By checking whether the second element is nonzero or not, it returns true when the value is a constant, false otherwise.
Inlining the IsConstant(constant) simply boils down to mov eax, 1 as shown here.

Such behavior is really useful when I deal with some SIMD optimization, as some instructions only take constant value.
It also helps me to replicate something like what System.Buffer.Memmove does for small constant lengths, without modifying the CoreCLR.
But it's an undefined behavior, and I'm not sure if it works with ARM-based processors, so I propose the APIs below for official support.

API Proposal

namespace System.Runtime.CompilerServices;

public static class Unsafe
{
    public static bool IsConstant<T>(T value) where T : unmanaged;
}

API Usage

// The method takes `[ConstantExpected] int index` and `Vector512<ushort> zmm2` as parameter
if (Unsafe.IsConstant(index))
{
    // some processing that needs the index to be a constant value
    return zmm2.GetElement(index);
}
// some processing that don't need the index to be a constant value
return Avx512BW.PermuteVar32x16(zmm2, Vector512.CreateScalarUnsafe((ushort)index)).GetElement(0);

Alternative Designs

  • There may be a better place for IsConstant to be.
  • There may be a better name for IsConstant.

Risks

None

Author: MineCake147E
Assignees: -
Labels:

api-suggestion, area-CodeGen-coreclr

Milestone: -

@EgorBo
Copy link
Member

EgorBo commented Feb 8, 2024

Your API Usage example sounds like something JIT should optimize itself when index is constant without extra help? Do you have other examples where this could be useful?

We already have an internal API for it called RuntimeHelpers.IsKnownConstant but I am not sure it should be exposed in public surface considering that it's quite fragile, e.g. #89379 we'd prefer improving JIT rather than exposing low-level hacky APIs to assist it.

@MineCake147E
Copy link
Contributor Author

Do you have other examples where this could be useful?

There are several cases where inlining the conditions for constant is fine as RyuJIT eliminates known conditional jumps, but executing multiple conditional jumps for variable cases would be tedious works for CPU.
Something like RuntimeHelpers.IsKnownConstant should be helpful in such cases, since I can use two completely different approaches, one for constants, one for variables.
One example in dotnet/runtime is System.Buffer.Memmove(ref byte dest, ref byte src, nuint len) which ended up using [Intrinsic] for unrolling constant cases, while using loop-based approach for variable cases.
It would be very difficult to solve these problems with JIT improvements alone.

We already have an internal API for it called RuntimeHelpers.IsKnownConstant but I am not sure it should be exposed in public surface considering that it's quite fragile, e.g. #89379 we'd prefer improving JIT rather than exposing low-level hacky APIs to assist it.

Exposing something like IsConstant<T>(T value) should obviously be much better than Vector128.CreateScalarUnsafe(value - ~value).GetElement(1) != 0 for both maintainability and performance, since IsConstant(variable) would just be xor eax, eax, not even using vmovd.
For Dynamic PGO... I couldn't come up with another idea other than just assume the code inside the block to be as hot as anything could be, or just assume the code inside the block doesn't even exist, unless inlining kicks in.
I would just slap [MethodImpl(MethodImplOptions.AggressiveInlining | MethodImplOptions.AggressiveOptimization)] on the method using it anyway, though.

@colejohnson66
Copy link

Just my two cents: "is known constant" is a weird method name. I get what it's implying: not all constants are "known" when inlining occurs, but it's one you have to think about. Should an API be exposed, I'd vote for "is inlined constant".

@tannergooding tannergooding added area-System.Runtime.CompilerServices and removed area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI labels Feb 9, 2024
@ghost
Copy link

ghost commented Feb 9, 2024

Tagging subscribers to this area: @dotnet/area-system-runtime-compilerservices
See info in area-owners.md if you want to be subscribed.

Issue Details

Background and motivation

[MethodImpl(MethodImplOptions.AggressiveInlining | MethodImplOptions.AggressiveOptimization)]
public static bool IsConstant(int value)
    => Vector128.CreateScalarUnsafe(value - ~value).GetElement(1) != 0;
    L0000: vzeroupper
    L0003: mov eax, ecx
    L0005: not eax
    L0007: sub ecx, eax
    L0009: vmovd xmm0, ecx
    L000d: vpextrd eax, xmm0, 1
    L0013: test eax, eax
    L0015: setne al
    L0018: movzx eax, al
    L001b: ret

The code above currently allows me to guess whether the value is a JIT-time constant in x86-64 environments.
The Vector128.CreateScalarUnsafe(value) usually does the same thing with Vector128.CreateScalar(value) (which is vmovd xmm*, r*d) which clears all upper elements with 0, but it does Vector128.Create(value) which broadcasts the value, when the value is a constant.
The value - ~value is guaranteed to be nonzero, as LLVM says, and it obviously differs by value.
By checking whether the second element is nonzero or not, it returns true when the value is a constant, false otherwise.
Inlining the IsConstant(constant) simply boils down to mov eax, 1 as shown here.

Such behavior is really useful when I deal with some SIMD optimization, as some instructions only take constant value.
It also helps me to replicate something like what System.Buffer.Memmove does for small constant lengths, without modifying the CoreCLR.
But it's an undefined behavior, and I'm not sure if it works with ARM-based processors, so I propose the APIs below for official support.

API Proposal

namespace System.Runtime.CompilerServices;

public static class Unsafe
{
    public static bool IsConstant<T>(T value) where T : unmanaged;
}

API Usage

// The method takes `[ConstantExpected] int index` and `Vector512<ushort> zmm2` as parameter
if (Unsafe.IsConstant(index))
{
    // some processing that needs the index to be a constant value
    return zmm2.GetElement(index);
}
// some processing that don't need the index to be a constant value
return Avx512BW.PermuteVar32x16(zmm2, Vector512.CreateScalarUnsafe((ushort)index)).GetElement(0);

Alternative Designs

  • There may be a better place for IsConstant to be.
  • There may be a better name for IsConstant.

Risks

None

Author: MineCake147E
Assignees: -
Labels:

api-suggestion, area-System.Runtime.CompilerServices, untriaged

Milestone: -

@tannergooding tannergooding added blocked Issue/PR is blocked on something - see comments and removed untriaged New issue has not been triaged by the area owner labels Feb 9, 2024
@tannergooding
Copy link
Member

Moved to the right area, but noting this is blocked on the JIT team signing off on it being a good thing to expose.

As @EgorBo has pointed out, there are limitations with this and there are many things it won't catch or handle. That being said, it could still be useful in the same ways it's been useful to us in the BCL.

@jkotas may also have opinions on this.

@jkotas
Copy link
Member

jkotas commented Feb 9, 2024

We had this method as internal API to experiment with for number of years, it has proven to be difficult to use. It is used in 6 places total currently, and 2 of those are being deleted in #98186.

@jkotas
Copy link
Member

jkotas commented Feb 9, 2024

System.Buffer.Memmove does for small constant lengths

Egor tried to optimize Memmove using this API in #83638, but it was too difficult to make it work. The intrinsic JIT expansion was implemented instead.

@EgorBo
Copy link
Member

EgorBo commented Feb 9, 2024

Right, here is that fully managed unrolling (for String.Equals and String.StartsWith): https://github.com/dotnet/runtime/pull/64821/files

  1. Inliner's budget quickly runs out
  2. You rely on all constant foldings working well (a fragile assumption)
  3. IsKnownConstant increases IL size and JIT currently can't precisely predict it will be able to fold branches like if (IsKnownConstant(str) && str.Length == 42)

But it was a funny experiment 🙂 E.g.:

image

@MichalPetryka
Copy link
Contributor

I'd guess one reasonable use case would be for checking inputs in SIMD code that then dispatches either to fast paths with ConstantExpected or slower paths for variable data.

@tannergooding
Copy link
Member

Going to close this based on the above.

Getting targeted JIT improvements around inlining, constant propagation, and potentially DPGO, are likely to be better long goals; particularly given the extreme limitations that the API in question has.

@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
api-suggestion Early API idea and discussion, it is NOT ready for implementation area-System.Runtime.CompilerServices blocked Issue/PR is blocked on something - see comments
Projects
None yet
Development

No branches or pull requests

6 participants