[release/8.0-staging] Ensure that the various Max*Number and Min*Number APIs optimize correctly
#98159
Conversation
|
Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch Issue DetailsBackport of #98125 to release/8.0-staging /cc @tannergooding Customer Impact
[Select one or both of the boxes. Describe how this issue impacts customers, citing the expected and actual behaviors and scope of the issue. If customer-reported, provide the issue number.] Regression
[If yes, specify when the regression was introduced. Provide the PR or commit if known.] Testing[How was the fix verified? How was the issue missed previously? What tests were added?] Risk[High/Medium/Low. Justify the indication by mentioning how risks were measured and addressed.] IMPORTANT: If this backport is for a servicing release, please verify that:
|
|
CC. @EgorBo for sign-off on this backport (since you signed off on the fix. This was an accidental regression in .NET 8, is customer reported, and the fix is simple/low risk. So it meets the requirements for backport. |
jeffschwMSFT
left a comment
There was a problem hiding this comment.
approved. we will take for consideration in 8.0.x
|
CC. @EgorBo, @dotnet/jit-contrib for merging. Monday February 12th is the Code Complete deadline for the March Release, so it needs to be merged by then (Julie asked that I let one of the JIT reviewers do the actual merge here). |
Backport of #98125 to release/8.0-staging
/cc @tannergooding
Customer Impact
Developers calling
MaxNumberorMinNumberwith one constant input may seeNaNpropagated instead of the number. This results in incorrect results as these APIs are explicitly meant to not propagate the NaN.Regression
This was introduced in .NET 8 as part of AVX-512 related acceleration work. The actual underlying issue was simple: we were swapping the operands as intended, but then re-swapped them back when actually modifying the node.
Testing
Tests were added covering the scenario in a way that explicitly ensures the JIT can only see one of the inputs as constant. If the JIT happened to see both inputs as constant, such as due to other optimizations, the value would have been correct.
Risk
Low. The issue was a simple one that was missed in code review and testing due to involving partial constant folding. It impacts a small set of math APIs, which themselves are relatively new to .NET (new in .NET 7)