Public API Proposal: Make bool RuntimeHelpers.IsKnownConstant(...) public for all primitive types #67285

gerhard17 · 2022-03-29T09:31:30Z

Background and motivation

Recently two internal IsKnownConstant() overloads in class RuntimeHelpers for char and string? were added.
For a library developer it would be usefull to consume this API like the framework internaly does.

Same performance/codegen reasons apply as with the original (internal) proposal. #11484

At least one overload for each primitive type (+ the existing string) would be wellcomed.

API Proposal

namespace System.Runtime.CompilerServices
{
    public static partial class RuntimeHelpers
    {
        // The following intrinsics return true if input is a compile-time constant
        // Feel free to add more overloads on demand

       // CHANGE VISIBILITY

        [Intrinsic]
        public static bool IsKnownConstant(string? t) => false;

        [Intrinsic]
        public static bool IsKnownConstant(char t) => false;

        // ADD OVERLOADS

        [Intrinsic]
        public static bool IsKnownConstant(byte t) => false;

        [Intrinsic]
        public static bool IsKnownConstant(sbyte t) => false;

        [Intrinsic]
        public static bool IsKnownConstant(short t) => false;

        [Intrinsic]
        public static bool IsKnownConstant(ushort t) => false;

        [Intrinsic]
        public static bool IsKnownConstant(int t) => false;

        [Intrinsic]
        public static bool IsKnownConstant(uint t) => false;

        [Intrinsic]
        public static bool IsKnownConstant(long t) => false;

        [Intrinsic]
        public static bool IsKnownConstant(ulong t) => false;

        [Intrinsic]
        public static bool IsKnownConstant(nint t) => false;

        [Intrinsic]
        public static bool IsKnownConstant(nuint t) => false;

        [Intrinsic]
        public static bool IsKnownConstant(float t) => false;

        [Intrinsic]
        public static bool IsKnownConstant(double t) => false;

    }
}

API Usage

API can be public used like the current internal usage inside of the runtime.

Alternative Designs

Maybe a true generic Version can be used.

namespace System.Runtime.CompilerServices
{
    public static partial class RuntimeHelpers
    {
        [Intrinsic]
        public static bool IsKnownConstant<T>(T t) => false;
    }
}

Risks

No breaking change. Not an public API today.
Will depend on JIT beyond c# language spec.

dotnet-issue-labeler · 2022-03-29T09:31:34Z

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

teo-tsirpanis · 2022-03-29T10:36:26Z

I'm in favor of making it generic.

ghost · 2022-03-29T10:36:40Z

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

Recently two internal IsKnownConstant() overloads in class RuntimeHelpers for char and string? were added.
For a library developer it would be usefull to consume this API like the framework internaly does.

Same performance/codegen reasons apply as with the original (internal) proposal. #11484

At least one overload for each primitive type (+ the existing string) would be wellcomed.

API Proposal

namespace System.Runtime.CompilerServices
{
    public static partial class RuntimeHelpers
    {
        // The following intrinsics return true if input is a compile-time constant
        // Feel free to add more overloads on demand

       // CHANGE VISIBILITY

        [Intrinsic]
        public static bool IsKnownConstant(string? t) => false;

        [Intrinsic]
        public static bool IsKnownConstant(char t) => false;

        // ADD OVERLOADS

        [Intrinsic]
        public static bool IsKnownConstant(byte t) => false;

        [Intrinsic]
        public static bool IsKnownConstant(sbyte t) => false;

        [Intrinsic]
        public static bool IsKnownConstant(short t) => false;

        [Intrinsic]
        public static bool IsKnownConstant(ushort t) => false;

        [Intrinsic]
        public static bool IsKnownConstant(int t) => false;

        [Intrinsic]
        public static bool IsKnownConstant(uint t) => false;

        [Intrinsic]
        public static bool IsKnownConstant(long t) => false;

        [Intrinsic]
        public static bool IsKnownConstant(ulong t) => false;

        [Intrinsic]
        public static bool IsKnownConstant(nint t) => false;

        [Intrinsic]
        public static bool IsKnownConstant(nuint t) => false;

        [Intrinsic]
        public static bool IsKnownConstant(float t) => false;

        [Intrinsic]
        public static bool IsKnownConstant(double t) => false;

    }
}

API Usage

API can be public used like the current internal usage inside of the runtime.

Alternative Designs

Maybe a true generic Version can be used.

namespace System.Runtime.CompilerServices
{
    public static partial class RuntimeHelpers
    {
        [Intrinsic]
        public static bool IsKnownConstant<T>(T t) => false;
    }
}

Risks

No breaking change. Not an public API today.
Will depend on JIT beyond c# language spec.

Author:	gerhard17
Assignees:	-
Labels:	`api-suggestion`, `area-System.Runtime.CompilerServices`, `untriaged`
Milestone:	-

SupinePandora43 · 2022-03-29T11:04:36Z

I'm in favor of making it generic.

Will it be able to consume null reference types and default value types?

EgorBo · 2022-03-29T11:49:01Z

The generic version must be guarded with : unmanaged constraint because with ref types it might introduce a runtime lookup that will prevent inlining
Is __builtin_constant popular in the C/C++ world? I've never seen it in the wild. Do you have a real-world scenario where it'd help you?
There are unresolved concerns with this API and JIT's inliner, e.g.:

void DoWork(int a)
{
    if (RuntimeHelpers.IsKnownConstant(a) && a == 42)
    {
        // a lot of code
    }
    // a lot of code
}

jit (currently) is not able to give it additional inlining boost only when a == 42 (currently it doesn't boost at all)

gerhard17 · 2022-03-29T12:46:01Z

@EgorBo

I would be fine with restriction unmanaged.
I found the C/C++ documentation during internet search. I just searched, if the JIT already implements such a thing. I for myself being somehow familiar with C/C++, I never used it in production code. :-)
Yes, currently I have such a case in an arbitrary presision floating point library. Common numeric constants given like literals would greatly benefit from this.
Yes this is similar to my use case. Only a little bit longer switch statement like:

[MethodImpl(MethodImplOptions.AggressiveInlining)]
void DoWork(int a)
{
    if (RuntimeHelpers.IsKnownConstant(a))
    {
        switch(a) {
        case 42:
          // some code
          return;
        case 43:
          // some code
          return;
         // some more cases with some code
       }
    }
    // a lot of generalized, but slower code
}

Btw. thanks for all the cool coding I found by you in this repository!

gerhard17 · 2022-03-29T12:47:50Z

Could restriction unmanaged be dangerous with large structs?
Copying struct data, when no JIT is involved.
Contradicting the performance gain in non JIT environments.

SupinePandora43 · 2022-03-29T12:52:34Z

Could restriction unmanaged be dangerous with large structs? Copying struct data, when no JIT is involved. Contradicting the performance gain in non JIT environments.

What about in and ref readonly?

jkotas · 2022-03-29T13:43:08Z

unresolved concerns with this API and JIT's inliner

Yep, it would be useful to see this API used in libraries in more places to prove that it actually works except for a few trivial carefully crafted cases.

#64821 was unsuccessfully in using this API for more complex code.

buyaa-n · 2022-03-30T18:03:49Z

Tag @GrabYourPitchforks for triage as he created the internal API proposal

gerhard17 · 2022-03-31T09:49:32Z

@jkotas
How does the annotation [MethodImpl(MethodImplOptions.AggressiveInlining)] change the inlining behavior?

Will all code be inlined or is only the budget increased to inline more (but maybe not all) code?

EgorBo · 2022-03-31T10:31:34Z

@jkotas How does the annotation [MethodImpl(MethodImplOptions.AggressiveInlining)] change the inlining behavior?

Will all code be inlined or is only the budget increased to inline more (but maybe not all) code?

Yes, large AggressiveInlining calls may eat the inliner budget pretty quickly - that is something we plan to fix eventually - I mean inliner should be smarter and do not take large pieces (branches/blocks) of methods which will be eliminated at the import stage into account, just not sure it will make it to .NET 7.0

Example: sharplab.io

GrabYourPitchforks · 2022-05-11T18:01:21Z

Tag GrabYourPitchforks for triage as he created the internal API proposal

I agree with Jan's assessment at #67285 (comment). This is experimental and very temperamental. We need more evidence that it's widely useful before exposing it publicly. It honestly wouldn't surprise me if the JIT folks somehow come up with some clever scheme which obviates the utility of IsKnownConstant entirely and we end up removing the API as a result.

redknightlois · 2024-05-22T11:51:58Z

This code should have a IsKnownConstant guard in the size. Currently we do it by code inspection. https://github.com/ravendb/ravendb/blob/c0bbece37e7b3d3afff2010b6b0e108633fd136e/src/Sparrow/Memory.cs#L472

We have a few examples on our codebase that could use a generic version.

gerhard17 added the api-suggestion Early API idea and discussion, it is NOT ready for implementation label Mar 29, 2022

dotnet-issue-labeler bot added the untriaged New issue has not been triaged by the area owner label Mar 29, 2022

gerhard17 changed the title ~~Public API suggestion: Make bool RuntimeHelpers.IsKnownConstant(...) public for all primitive types~~ Public API Proposal: Make bool RuntimeHelpers.IsKnownConstant(...) public for all primitive types Mar 29, 2022

teo-tsirpanis added the area-System.Runtime.CompilerServices label Mar 29, 2022

joperezr added this to the Future milestone May 4, 2022

ghost removed the untriaged New issue has not been triaged by the area owner label May 4, 2022

jakobbotsch mentioned this issue May 11, 2022

Optimize Span.Slice by using IsKnownConstant for start = 0 #69193

Closed

pentp mentioned this issue Sep 26, 2022

[RyuJIT] Div then Mod (or viceversa) executes idiv twice instead of using the value in the register #5213

Open

2A5F mentioned this issue Sep 14, 2024

[API Proposal]: Determine whether the passed parameter is a constant #107822

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Public API Proposal: Make bool RuntimeHelpers.IsKnownConstant(...) public for all primitive types #67285

Public API Proposal: Make bool RuntimeHelpers.IsKnownConstant(...) public for all primitive types #67285

gerhard17 commented Mar 29, 2022 •

edited

Loading

dotnet-issue-labeler bot commented Mar 29, 2022

teo-tsirpanis commented Mar 29, 2022

ghost commented Mar 29, 2022

Background and motivation

API Proposal

API Usage

Alternative Designs

Risks

SupinePandora43 commented Mar 29, 2022

EgorBo commented Mar 29, 2022 •

edited

Loading

gerhard17 commented Mar 29, 2022 •

edited

Loading

gerhard17 commented Mar 29, 2022 •

edited

Loading

SupinePandora43 commented Mar 29, 2022

jkotas commented Mar 29, 2022 •

edited by EgorBo

Loading

buyaa-n commented Mar 30, 2022

gerhard17 commented Mar 31, 2022

EgorBo commented Mar 31, 2022 •

edited

Loading

GrabYourPitchforks commented May 11, 2022

redknightlois commented May 22, 2024 •

edited

Loading

Public API Proposal: Make bool RuntimeHelpers.IsKnownConstant(...) public for all primitive types #67285

Public API Proposal: Make bool RuntimeHelpers.IsKnownConstant(...) public for all primitive types #67285

Comments

gerhard17 commented Mar 29, 2022 • edited Loading

Background and motivation

API Proposal

API Usage

Alternative Designs

Risks

dotnet-issue-labeler bot commented Mar 29, 2022

teo-tsirpanis commented Mar 29, 2022

ghost commented Mar 29, 2022

Background and motivation

API Proposal

API Usage

Alternative Designs

Risks

SupinePandora43 commented Mar 29, 2022

EgorBo commented Mar 29, 2022 • edited Loading

gerhard17 commented Mar 29, 2022 • edited Loading

gerhard17 commented Mar 29, 2022 • edited Loading

SupinePandora43 commented Mar 29, 2022

jkotas commented Mar 29, 2022 • edited by EgorBo Loading

buyaa-n commented Mar 30, 2022

gerhard17 commented Mar 31, 2022

EgorBo commented Mar 31, 2022 • edited Loading

GrabYourPitchforks commented May 11, 2022

redknightlois commented May 22, 2024 • edited Loading

gerhard17 commented Mar 29, 2022 •

edited

Loading

EgorBo commented Mar 29, 2022 •

edited

Loading

gerhard17 commented Mar 29, 2022 •

edited

Loading

gerhard17 commented Mar 29, 2022 •

edited

Loading

jkotas commented Mar 29, 2022 •

edited by EgorBo

Loading

EgorBo commented Mar 31, 2022 •

edited

Loading

redknightlois commented May 22, 2024 •

edited

Loading