Skip to content

Fix Avx.Compare miscompile with non-constant FloatComparisonMode - #131482

Open
tannergooding wants to merge 1 commit into
dotnet:mainfrom
tannergooding:tannergooding-fix-avx-compare-nonconst-mode
Open

Fix Avx.Compare miscompile with non-constant FloatComparisonMode#131482
tannergooding wants to merge 1 commit into
dotnet:mainfrom
tannergooding:tannergooding-fix-avx-compare-nonconst-mode

Conversation

@tannergooding

Copy link
Copy Markdown
Member

Fixes #131472.

Avx.Compare / Avx512F.Compare produced garbage or AV'd under FullOpts when the FloatComparisonMode argument was not a compile-time constant (e.g. a loop induction variable). It reproduces on shipped 11.0.0-preview.6 with DOTNET_TieredCompilation=0; tiering masked it.

Root cause

A non-constant mode has no register-variable encoding, so Compare defers to its managed fallback via a user call (setMethodHandle in impHWIntrinsic). On EVEX-capable hardware the intrinsic is promoted to the mask-returning form and wrapped in a ConvertMaskToVector node. The shared attach block tagged that unary wrapper rather than the inner 3-operand compare, so the rationalizer built a call carrying only leftright/mode were never set up, and the call fell through to the self-recursive managed body on uninitialized registers.

512-bit is affected for the same reason and cannot avoid the mask: NI_AVX512_Compare carries HW_Flag_InvalidNodeId, so the mask form is mandatory.

Fix

In the rare setMethodHandle path, when the return node is a ConvertMaskToVector, attach the fallback handle to its inner mask operand instead of the wrapper. This runs only for the non-constant-imm-under-optimization case (no common-path cost) and covers 128/256-bit EVEX and mandatory-mask 512-bit uniformly. ConvertMaskToVector is unary on both xarch and arm64, so Op(1) is unambiguous; asserts guard against future wrong-node tagging.

Testing

  • New JitBlue regression test (GitHub_131472) covering Vector256<float> (optional EVEX promotion) and Vector512<float> (mandatory mask), comparing FullOpts against a MinOpts reference over all 32 modes. Fails without the fix, passes with it.
  • Constant-mode Compare codegen is provably unchanged (the new code only executes when setMethodHandle is set, which never happens for a constant mode); verified correct results.
  • Checked and Release JIT build clean; jitformat clean.

Note

This change was authored with the assistance of GitHub Copilot.

A non-constant mode makes Compare defer to its managed fallback via a user call.
When the intrinsic is promoted to the mask-returning form, the result is wrapped
in a mask-to-vector conversion, so attaching the fallback handle to that wrapper
left the rewritten call missing its right/mode operands. Attach the handle to the
inner mask node instead.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings July 28, 2026 17:42
@github-actions github-actions Bot added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Jul 28, 2026
@tannergooding

Copy link
Copy Markdown
Member Author

@JulieLeeMSFT this is a bug fix (pre-existing issue, not new this release) and should be considered for .NET 11

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 5 pipeline(s).
11 pipeline(s) were filtered out due to trigger conditions.
There may be pipelines that require an authorized user to comment /azp run to run.

@dotnet-policy-service

Copy link
Copy Markdown
Contributor

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

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR aims to fix a CoreCLR JIT miscompile in the Avx.Compare / Avx512F.Compare non-constant FloatComparisonMode path (FullOpts on EVEX-capable hardware) by adjusting where the JIT attaches the “rewrite as user call” method handle, and adds a JIT regression test to cover the scenario.

Changes:

  • Update impHWIntrinsic to retarget SetMethodHandle from a ConvertMaskToVector wrapper to its inner operand.
  • Add a new JitBlue regression test for variable FloatComparisonMode over all 32 modes for Vector256<float> and Vector512<float>.
  • Include the new regression test source in the Regression_ro_2.csproj compilation list.
Show a summary per file
File Description
src/coreclr/jit/hwintrinsic.cpp Changes method-handle attachment logic for late-stage intrinsic→user-call rewriting when a mask-to-vector wrapper is present.
src/tests/JIT/Regression/JitBlue/GitHub_131472/GitHub_131472.cs Adds regression coverage comparing FullOpts vs NoOptimization results across all FloatComparisonMode values for AVX/AVX512.
src/tests/JIT/Regression/Regression_ro_2.csproj Wires the new test into the merged regression project build.

Copilot's findings

  • Files reviewed: 3/3 changed files
  • Comments generated: 1

Comment thread src/coreclr/jit/hwintrinsic.cpp
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI

Projects

None yet

Development

Successfully merging this pull request may close these issues.

JIT: Avx.Compare/Avx.CompareScalar miscompile (AccessViolationException) with a non-constant FloatComparisonMode on EVEX-capable hardware

2 participants