Skip to content

Conversation

@MichalStrehovsky
Copy link
Member

  • Program.cs: if AVX instruction set was selected as a minimum requirement, allow AvxVnni under a runtime check.
  • HardwareIntrinsicHelpers.Aot.cs: Add support for dynamic IsSupported checks for AvxVnni. Also if AvxVnni was selected as a requirement, burn the requirement into the executable so that we can check it at startup.
  • Startup.cpp: check for AvxVnni so that we can use it in IsSupported checks and also fail to start if it's a hard requirement.
  • Add a test for the NativeAOT-specific parts of the matrix.

* Program.cs: if AVX instruction set was selected as a minimum requirement, allow AvxVnni under a runtime check.
* HardwareIntrinsicHelpers.Aot.cs: Add support for dynamic `IsSupported` checks for AvxVnni. Also if AvxVnni was selected as a requirement, burn the requirement into the executable so that we can check it at startup.
* Startup.cpp: check for AvxVnni so that we can use it in `IsSupported` checks and also fail to start if it's a hard requirement.
* Add a test for the NativeAOT-specific parts of the matrix.
@MichalStrehovsky
Copy link
Member Author

Cc @tannergooding - I missed when AvxVnni was added - this is the extent of NativeAOT changes needed to support new HW intrinsics. Basically, just dealing with the fact that IsSupported can have 3 possible states in NativeAOT - always supported, supported under a runtime check, and always unsupported.

optimisticInstructionSetSupportBuilder.AddSupportedInstructionSet("fma");
optimisticInstructionSetSupportBuilder.AddSupportedInstructionSet("bmi");
optimisticInstructionSetSupportBuilder.AddSupportedInstructionSet("bmi2");
optimisticInstructionSetSupportBuilder.AddSupportedInstructionSet("avxvnni");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is going to add AVX2 to the list. Is that problematic?

My guess is no since we only use the supportedInstructionSet flags to determine things like Vector<T> length, but seeing as AVX2 is already "missing" here, I thought I'd ask.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, AVX2 is probably an omission. I don't think there's a reason why we wouldn't be able to support it under the runtime check.

@MichalStrehovsky
Copy link
Member Author

The failure is #63561 (unrelated).

@jkotas jkotas merged commit 5f80aca into dotnet:main Jan 10, 2022
@MichalStrehovsky MichalStrehovsky deleted the avxvnni branch January 11, 2022 00:36
@ghost ghost locked as resolved and limited conversation to collaborators Feb 10, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants