Update the naot instruction set support for .NET 10+ #2828
Conversation
| { | ||
| if (IsX86Avx512Supported) | ||
| { | ||
| return "x86-64-v4"; |
There was a problem hiding this comment.
These are the official short names for the ISA groupings.
There was a problem hiding this comment.
While I am very happy about the fix and other improvements, I find myself hesitant about the new "display name":
- most engineers know what AVX is, I am not sure about the ISA groupings (at least I was not familiar with those names so far)
- the ISA groupings contain information that is already displayed on the screen: the architecture
BenchmarkDotNet v0.15.3-develop (2025-09-11), Windows 11 (10.0.26100.6584/24H2/2024Update/HudsonValley)
AMD Ryzen Threadripper PRO 3945WX 12-Cores 3.99GHz, 1 CPU, 24 logical and 12 physical cores
.NET SDK 10.0.100-preview.6.25358.103
- [Host] : .NET 8.0.20 (8.0.20, 8.0.2025.41914), X64 RyuJIT AVX2
+ [Host] : .NET 8.0.20 (8.0.20, 8.0.2025.41914), X64 RyuJIT x86-64-v3@AndreyAkinshin thoughts?
There was a problem hiding this comment.
I have the same concern: AVX2 is much more convinient for the users unlike x86-64-v3. Since we have enough space in this line, I'm suggesting to print both options.
There was a problem hiding this comment.
I'm in a bit of a disagreement here.
The names were formalized several years ago by Intel, AMD, LLVM, and others explicitly because it was becoming too unwieldy. Both to list all the options and because of the rapidly exploding number of combinations that could be specified. The manufacturers largely do not want developers thinking in terms of individual ISAs so much anymore and want them to essentially target "profiles" instead, for simplicity.
This also follows the general model that Arm64 and other manufacturers use as well. You declare your base profile (i.e. armv8.0-a) and then optionally list any key extensions on top (i.e. armv8.0-a + lse). So the intent is that you squash the complex profiles together (i.e. x86-64-v4) and then list key important extensions that aren't part of a defined profile on top (i.e. x86-64-v4 + APX).
This is likewise how we've changed the JIT to largely support the feature sets, how the JIT Dumps and JIT Disasm now print the information, and how the configuration knobs for manual disablement work.
We explicitly want developers to stop thinking in terms of "AVX2" or "AVX512" in the common cases, but they still have the ability to print the full ISA list if desired (via GetFullInfo)
There was a problem hiding this comment.
I have no objections to using the new notation. However, I want us to maintain backward compatibility with user habits.
We explicitly want developers to stop thinking in terms of "AVX2" or "AVX512" in the common cases
BenchmarkDotNet does not have this goal. In this project, we want to provide a convenient user experience so that people can quickly gain insights from the information presented based on their existing knowledge. If we decide that we want to promote the new notation in addition to a convenient user experience, it makes even more sense to display both versions. This approach will naturally help people create a mental map without needing to search for details when they encounter unfamiliar labels. We can use the new notation in the first place, but then explicitly specify instruction sets inside.
There was a problem hiding this comment.
Isn’t it fairly counterintuitive to go against what the hardware manufacturers, compiler developers, and underlying platform have all agreed is the best for long term UX?
This wasn’t some arbitrary decision, but the result of extensive discussion based around real world pain points and confusion caused by the rapidly expanding complexity of ISAs
———-
Part of the consideration is that typical users don’t understand what things like AVX2 are and those that roughly know often get it confused for meaning the wrong thing. It’s only the people that directly work with intrinsics that actually understand
This is very similar in concept to people understanding the brand name “Windows 8” or “Windows 11” but not understanding how that maps to version numbers (6.2 and 10.0.22000)
The profiles are meant to be explicitly clear and give a point that shows something as being meaningfully greater without getting into the technical nuance that is irrelevant to the typical case — I.e listing things that won’t actually impact codegen or make a meaningful difference; which is an actual “problem” with how BDN lists ISAs today, surfacing info that likely isn’t causing profiling differences by default to typical devs
Particularly with how .NET actually works, this also applies here and simplifies what devs should be considering without removing pertinent info. They see the different baselines we support and do codegen differentiation against, nothing more unless they ask for details
———-
I strongly expect that the distaste here is more an initial visceral reaction to the “cheese being moved” from devs that do have more technical knowledge of the space. Such a reaction would likely be less strong if you had less knowledge of the space or were more ingrained with the space and kept up to date with the best practices and recommendations for dealing with hardware profiles as they’ve evolved over the past 5-10 years (not just for x64 but also for Arm64 and other major cpu vendors who are all unifying on such approaches)
There was a problem hiding this comment.
OK, you convinced me; let's try your way. =)
I still expect that it will cause confusion for some of our users, but it's probably not a big problem.
| if (IsX86PclmulqdqSupported) yield return "PCLMUL"; | ||
| if (IsX86PopcntSupported) yield return "POPCNT"; | ||
| { | ||
| if (IsX86Avx10v2Supported) yield return "AVX10v2"; |
There was a problem hiding this comment.
This prints the full set of ISAs supported based on the way we group them in RyuJIT
| return avx512.ToString(); | ||
| } | ||
|
|
||
| #pragma warning disable CA2252 // Some APIs require opting into preview features | ||
| internal static bool IsX86BaseSupported => |
There was a problem hiding this comment.
These are grouped based on the DOTNET_Enable* flags we expose so that we're only querying what can actually be toggled on/off.
Co-authored-by: Tim Cassell <[email protected]>
|
Still getting compile errors. PTAL |
| { | ||
| if (IsX86Avx512Supported) | ||
| { | ||
| return "x86-64-v4"; |
There was a problem hiding this comment.
While I am very happy about the fix and other improvements, I find myself hesitant about the new "display name":
- most engineers know what AVX is, I am not sure about the ISA groupings (at least I was not familiar with those names so far)
- the ISA groupings contain information that is already displayed on the screen: the architecture
BenchmarkDotNet v0.15.3-develop (2025-09-11), Windows 11 (10.0.26100.6584/24H2/2024Update/HudsonValley)
AMD Ryzen Threadripper PRO 3945WX 12-Cores 3.99GHz, 1 CPU, 24 logical and 12 physical cores
.NET SDK 10.0.100-preview.6.25358.103
- [Host] : .NET 8.0.20 (8.0.20, 8.0.2025.41914), X64 RyuJIT AVX2
+ [Host] : .NET 8.0.20 (8.0.20, 8.0.2025.41914), X64 RyuJIT x86-64-v3@AndreyAkinshin thoughts?
The instruction set support was simplified in the JIT for .NET 10 and so this updates the support to better match said updates while still trying to remain backwards compatible with .NET 6-9.
This resolves dotnet/runtime#119353