Skip to content

JIT: Preserve SIMD operand evaluation and ordering - #133776

Open
tannergooding wants to merge 3 commits into
dotnet:mainfrom
tannergooding:tannergooding-investigate-nan-min-max-folding
Open

JIT: Preserve SIMD operand evaluation and ordering#133776
tannergooding wants to merge 3 commits into
dotnet:mainfrom
tannergooding:tannergooding-investigate-nan-min-max-folding

Conversation

@tannergooding

Copy link
Copy Markdown
Member

Fixes #133718.

Preserve observable evaluation when SIMD folding discards an operand, including constant-NaN Min/Max. Keep operand evaluation and validation in source order through shift emulation, GetElement/WithElement, scalar-left multiplication, sequences, and value-first stores.

Use GTF_OBS_EFFECT to distinguish observable effects from unused ordinary reads. Carry binary evaluation order in the builders instead of relying on importer spills, including local-morph handling for reversed stores and unary local-store folding. Retain WASM, FMA, and decomposition spills where still required; avoid unnecessary captures for invariant single-lane results and direct int/uint division nodes. Also clone scalar Min/Max operands in source order on RISC-V.

This does not change NaN payload-selection policy or address the separate XCHG/local-value-interference problem.

Validation

  • Windows x64 and ARM64 Checked JIT builds; required Windows x64 JIT formatting passes.
  • All 23 focused regression cases pass with default ISA support and with AVX-512, AVX2, AVX, or hardware intrinsics disabled. The same assembly has 7 failures against the saved unfixed JIT.
  • All 2,551 optimized general-intrinsics cases pass with default ISA support and again with AVX-512 disabled.
  • Eleven x64 value/codegen controls pass against both JITs across four ISA configurations, with no size or instruction-count increases. The default Vector128<int>.CreateSequence array-load probe drops from 63 to 57 bytes and 16 to 14 instructions; direct integer-division and ordinary-store controls are unchanged. Seven pure Min/Max, Number, and finite controls have identical disassembly. These are codegen measurements, not throughput benchmarks.

ARM64 was compiled but not executed. WASM and RISC-V were not built or executed; the ARM64 single-lane optimizations were not measured natively.

Note

This PR description was drafted with GitHub Copilot.

tannergooding and others added 2 commits September 11, 2026 18:55
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Carry source evaluation order on two-operand nodes instead of relying on importer spills, and handle reversed stores during local morph. Retain spills where operand decomposition still requires stable values.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot AI lite review requested due to automatic review settings September 12, 2026 08:40
@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 Sep 12, 2026
@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.

@tannergooding

Copy link
Copy Markdown
Member Author

Ended up going through and looking at all the SIMD related places that might have similar issues and ensuring they're consistent and correct. This looked to be all of them, but it ended up growing the PR compared to the absolute minimal fix.

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.

Copilot review overview

🔵 Needs a closer look

Three unresolved moderate comments remain in gentree.cpp, requiring fixes and human review.

Review tier: Lite
Findings: None

What changed in this PR

This PR preserves SIMD operand evaluation order and observable effects during JIT folding and decomposition for issue #133718.

Changes:

  • Adds observable-effect tracking and ordering-aware SIMD builders.
  • Preserves evaluation order across shifts, element operations, stores, Min/Max folding, and platform-specific paths.
  • Adds regression coverage for affected evaluation-order cases.
File Summary
src/​tests/​JIT/​Regression_ro_2/​Runtime_133718.cs Adds SIMD evaluation-order regression tests.
src/​coreclr/​jit/​optimizer.cpp Uses observable-effect tracking during optimization.
src/​coreclr/​jit/​optimizebools.cpp Updates effect checks for boolean optimization.
src/​coreclr/​jit/​morph.cpp Preserves effects when folding operations.
src/​coreclr/​jit/​lclmorph.cpp Handles reversed evaluation order during local-store morphing.
src/​coreclr/​jit/​importercalls.cpp Preserves RISC-V Min/Max operand order.
src/​coreclr/​jit/​importer.cpp Updates inline argument effect tracking.
src/​coreclr/​jit/​ifconversion.cpp Uses observable effects during if-conversion.
src/​coreclr/​jit/​hwintrinsicxarch.cpp Updates xarch intrinsic effect handling.
src/​coreclr/​jit/​hwintrinsic.cpp Adjusts SIMD sequence, division, and store importing.
src/​coreclr/​jit/​gentree.h Defines observable-effect flags.
src/​coreclr/​jit/​gentree.cpp Implements ordering-aware SIMD folding and builders. Three moderate comments remain about materializing address-exposed operands before duplication.
src/​coreclr/​jit/​compiler.h Extends SIMD store-builder declarations.

Copilot AI review requested due to automatic review settings September 12, 2026 18:09

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.

Copilot review overview

🔵 Needs a closer look

Broad cross-platform JIT changes require final human review; several platforms were not executed and focused coverage remains requested.

Review tier: Lite
Findings: None

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: (bug) constant-NaN Math.Min/Max folding drops the other argument's side effects

2 participants