[release/6.0] Ensure Max/Min for floating-point on x86/x64 are not handled as commutative

[tannergooding]

Backport of #75683 to release/6.0

/cc @tannergooding

Customer Impact

Customers utilizing the Sse.Min/Max(float), Sse2.Min/Max(double) or Avx.Min/Max(float/double) intrinsics may get "invalid" codegen that results in an incorrect result being returned if an input is NaN or both inputs are zero (of any sign).

This issue was raised by ImageSharp as it was impacting their ability to update code to target .NET 7. It also exists in .NET 6 and is trivial to hit with a simple refactoring of existing their existing logic.

Testing

Explicit tests were added to cover the impacted scenarios.

Risk

Low. This issue has been in the codebase for at least 5 years and impacts .NET Core 3.1, 5.0, 6.0, and 7.0. Due to other codegen improvements, it is now more likely to be hit in .NET 7.

However, the root cause of the issue is related to an incorrect flag labeling the intrinsics as being commutative and we are effectively removing that flag so the code churn is small and any risk of the change is localized to these intrinsics.

[ghost]

Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch
See info in area-owners.md if you want to be subscribed.

Issue Details

---

Backport of #75683 to release/7.0-rc2

/cc @tannergooding

Customer Impact

Customers utilizing the Sse.Min/Max(float), Sse2.Min/Max(double) or Avx.Min/Max(float/double) intrinsics may get "invalid" codegen that results in an incorrect result being returned if an input is NaN or both inputs are zero (of any sign).

This issue was raised by ImageSharp as it was impacting their ability to update code to target .NET 7. It also exists in .NET 6 and is trivial to hit with a simple refactoring of existing their existing logic.

Testing

Explicit tests were added to cover the impacted scenarios.

Risk

Low. This issue has been in the codebase for at least 5 years and impacts .NET Core 3.1, 5.0, 6.0, and 7.0. Due to other codegen improvements, it is now more likely to be hit in .NET 7.

However, the root cause of the issue is related to an incorrect flag labeling the intrinsics as being commutative and we are effectively removing that flag so the code churn is small and any risk of the change is localized to these intrinsics.

Author:	tannergooding
Assignees:	-
Labels:	area-CodeGen-coreclr
Milestone:	-

[tannergooding]

Backported to .NET 6 as well as per the discussion on #75761

CC. @EgorBo

[JulieLeeMSFT]

Cc @jeffschwMSFT.

[jeffschwMSFT]

approved. we will take for consideration in 6.0.x

[carlossanlop]

@tannergooding @jeffschwMSFT the milestone is incorrect, this is a 6.0 backport. Is this fix critical for the October release? If yes, then I can merge it, if not, then we will have to wait till next month.

cc @mmitche

[jeffschwMSFT]

Thanks for the correction. I think this aligns with 6.0.11 (right?)

[carlossanlop]

Thanks for the correction. I think this aligns with 6.0.11 (right?)

6.0.10 is October and 6.0.11 is November IIRC. @tannergooding what do you think? Is this critical for October or not?

[carlossanlop]

Talked to @tannergooding via Teams. This can wait for the next release.

Also, this needs no OOB package authoring changes AFAIK, since it's modifying native code.

[carlossanlop]

Branch is open again. CI is green, approved, signed-off, no OOB package authoring required, ready to merge.