dotnet / runtime

.NET is a cross-platform runtime for cloud, mobile, desktop, and IoT apps.
https://docs.microsoft.com/dotnet/core/
MIT License
15.28k stars 4.73k forks source link

[Analyzer] Upgrade from platform specific to cross platform intrinsics #82486

Open tannergooding opened 1 year ago

tannergooding commented 1 year ago

We've iteratively exposed support for various levels of hardware intrinsics since .NET Core 3.1. In .NET 7, we exposed the new "cross platform" hardware intrinsics which aim to help simplify the writing and maintenance of SIMD code meant to run across a variety of platforms.

As such, we should provide a code fixer that identifies such platform specific calls and suggests the simple transformation to the equivalent cross platform calls.

Category: Maintainability or Portability seem like the best fit. I could also see this being classified as Usage Severity = suggestion

Example

A simple example is given below. The full list of APIs recognized is of course more expansive but effectively correlates to a platform specific ISA such as Sse.Add, Sse2.Add, Avx.Add, Avx2.Add, AdvSimd.Add, or PackedSimd.Add correlating to a singular cross platform API, such as operator +:

- return Sse.Add(Sse.Multiply(left, right), addend);
+ return (left * right) + addend;
ghost commented 1 year ago

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

Issue Details
We've iteratively exposed support for various levels of hardware intrinsics since .NET Core 3.1. In .NET 7, we exposed the new "cross platform" hardware intrinsics which aim to help simplify the writing and maintenance of SIMD code meant to run across a variety of platforms. As such, we should provide a code fixer that identifies such platform specific calls and suggests the simple transformation to the equivalent cross platform calls. Category: [Maintainability](https://learn.microsoft.com/en-us/dotnet/fundamentals/code-analysis/quality-rules/maintainability-warnings) or [Portability](https://learn.microsoft.com/en-us/dotnet/fundamentals/code-analysis/quality-rules/interoperability-warnings) seem like the best fit. I could also see this being classified as [Usage](https://learn.microsoft.com/en-us/dotnet/fundamentals/code-analysis/quality-rules/usage-warnings) [Severity = suggestion](https://learn.microsoft.com/en-us/dotnet/fundamentals/code-analysis/configuration-options#severity-level) ### Example A simple example is given below. The full list of APIs recognized is of course more expansive but effectively correlates to a platform specific ISA such as `Sse.Add`, `Sse2.Add`, `Avx.Add`, `Avx2.Add`, `AdvSimd.Add`, or `PackedSimd.Add` correlating to a singular cross platform API, such as `operator +`: ```diff - return Sse.Add(Sse.Multiply(left, right), addend); + return (left * right) + addend; ```
Author: tannergooding
Assignees: -
Labels: `area-System.Runtime.Intrinsics`, `code-analyzer`, `code-fixer`, `api-ready-for-review`
Milestone: -
terrajobst commented 1 year ago

Video

Code fixer looks good as proposed.

This would handle most common operators (+, -, /, *, and even Xor).

Either category seems fine, but we have a slight preference for maintainability.

- Sse.Add(Sse.Multiply(left, right), addend)
+ (left * right) + addend
tannergooding commented 1 year ago

@buyaa-n, I'd like to start prototyping this. What's required before I start doing that?

Do we need to determine which dotnet/roslyn-analyzers project this will be part of and "allocate" some diagnostic ID(s)?

buyaa-n commented 1 year ago

What's required before I start doing that?

Might want to start with the getting started doc, some info might be a bit old, let me know if you have any questions. Could check other PRs that adding a new analyzer as an example

Do we need to determine which dotnet/roslyn-analyzers project this will be part of and "allocate" some diagnostic ID(s)?

Libraries analyzers goes into Microsoft.NetCore.Analyzers project, for id just pick the next available ID for the category https://github.com/dotnet/roslyn-analyzers/blob/main/src/Utilities/Compiler/DiagnosticCategoryAndIdRanges.txt#L20. Looks last id used for Maintainability category is CA1513, so you can pick CA1514 and update the range in the file accordingly

TheBlackPlague commented 1 year ago

If it's okay, I'd like to pitch in — I'm the developer of StockNemo, a chess engine in C# that heavily makes use of SIMD intrinsics available for its neural network inference.

Please see: https://github.com/TheBlackPlague/StockNemo/blob/f3b52613049c03d8d2a85dfdba417481c6ef795f/Backend/Engine/NNUE/Vectorization/VMethod.cs#L43

Maybe when operations like matrix_o = matrix_x * matrix_w + matrix_b (neural network propagation operation), we can resort to using the fused multiply and add adjacent?

In some cases, this will work better than the regular operations. Let me know what you think.

tannergooding commented 1 year ago

Maybe when operations like matrix_o = matrix_x * matrix_w + matrix_b (neural network propagation operation), we can resort to using the fused multiply and add adjacent?

This isn't a "value-preserving optimization" so its not one we can offer by default. In particular, it will result in different results for "overflow" to +/-infinity and in slightly different results in a few other cases due to rounding once rather than twice.

If it's okay, I'd like to pitch in

I appreciate the offer. We're going to be looking at this closer to the Preview 5/6 timeframe and I'll ping again on the thread around that time.