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

Add [Immediate] attribute for parameters which should be constants #33814

Closed
GrabYourPitchforks opened this issue Mar 19, 2020 · 2 comments
Closed
Labels
api-suggestion Early API idea and discussion, it is NOT ready for implementation area-System.Runtime.CompilerServices code-analyzer Marks an issue that suggests a Roslyn analyzer

Comments

@GrabYourPitchforks
Copy link
Member

GrabYourPitchforks commented Mar 19, 2020

(Copied from #30740 (comment).)

We have scenarios (particularly in the Intrinsics namespace) where certain APIs expect constant values as parameters. For instance, Sse2.ShiftLeftLogical(Vector, byte) expects a constant value for its last argument, as the constant value needs to be encoded as part of the instruction. You can pass a non-constant, but if you do this you'll get redirected to a jump table, which could kill your performance. And presumably you were using intrinsics because you wanted bare-metal performance. :)

As a strawman for an analyzer that could detect this, we could consider an [Immediate] attribute that could be placed on parameters and an analyzer that would flag call sites where a non-const is passed in for these parameters. Bruce had further suggested we could extend it with min and max properties if needed.

So, using the SSE2 example mentioned earlier, the annotation would appear as follows.

namespace System.Runtime.Intrinsics.X86
{
    public abstract class Sse2 : Sse
    {
        public static Vector128<short> ShiftLeftLogical(Vector128<short> value, [Immediate( /* Min = 0, Max = 15 */)] byte count);
    }
}

Category: Performance (possibly also correctness?)

@GrabYourPitchforks GrabYourPitchforks added api-suggestion Early API idea and discussion, it is NOT ready for implementation area-System.Runtime.CompilerServices untriaged New issue has not been triaged by the area owner code-analyzer Marks an issue that suggests a Roslyn analyzer labels Mar 19, 2020
@tannergooding
Copy link
Member

Duplicate of #33771 ?

@GrabYourPitchforks
Copy link
Member Author

Derp. I somehow missed that he added that.

@ghost ghost locked as resolved and limited conversation to collaborators Dec 10, 2020
@tannergooding tannergooding removed the untriaged New issue has not been triaged by the area owner label Jun 24, 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 code-analyzer Marks an issue that suggests a Roslyn analyzer
Projects
None yet
Development

No branches or pull requests

2 participants