Skip to content

JIT: fix SIMD12 relocatable-address containment check in ContainCheckIndir - #132676

Merged
jakobbotsch merged 2 commits into
mainfrom
copilot/fix-simd12-containment-check
Aug 24, 2026
Merged

jakobbotsch merged 2 commits into
mainfrom
copilot/fix-simd12-containment-check

Conversation

Copilot AI commented Aug 24, 2026 •

Copy link
Copy Markdown
Contributor

Lowering::ContainCheckIndir on xarch tested addr->TypeIs(TYP_SIMD12), but addr is the address operand and is always pointer/byref typed — so the condition was vacuously true and the relocation restriction described in the comment right below it never applied.

 #if defined(FEATURE_SIMD)
-        if ((!addr->TypeIs(TYP_SIMD12) || !icon->ImmedValNeedsReloc(m_compiler)) && icon->FitsInAddrBase(m_compiler))
+        if ((!node->TypeIs(TYP_SIMD12) || !icon->ImmedValNeedsReloc(m_compiler)) && icon->FitsInAddrBase(m_compiler))
 #else

The restriction belongs on the indirection's type: genLoadIndTypeSimd12/genStoreIndTypeSimd12 reach the upper element via a second access at +8, which they implement by mutating a contained address constant (icon->SetIconValue(icon->IconValue() + 8)) under assert(!icon->ImmedValNeedsReloc(m_compiler)).

  • Scope: only affects compilation with JIT_FLAG_RELOC (AOT). For the runtime JIT ImmedValNeedsReloc is always false, so containment behavior is unchanged there.
  • Grep check: this was the only addr->TypeIs(TYP_SIMD12) occurrence in the JIT.

Verification

A NativeAOT compile (checked JIT) of a static Vector3 load/store hits this today:

ILC: Assertion failed '!icon->ImmedValNeedsReloc(m_compiler)' in 'Repro:Store(System.Numerics.Vector3)' during 'Generate code'

With the change the address constant is no longer contained and the upper-element access is correct:

lea       rax, [(reloc 0x4203c8)]
movsd     qword ptr [rax], xmm0
extractps dword ptr [rax+0x08], xmm0, 2

On regression tests

No test added. The path requires AOT relocations, so a JitBlue test cannot reach it; conversely any NativeAOT compilation of a static Vector3 store trips the assert under a checked JIT, so existing NativeAOT legs already cover the class broadly. Happy to add an explicit test under src/tests/nativeaot/ if reviewers prefer one.

@azure-pipelines

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

@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 Aug 24, 2026
@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 changed the title [WIP] Fix incorrect SIMD12 containment check in Lowering::ContainCheckIndir JIT: fix SIMD12 relocatable-address containment check in ContainCheckIndir Aug 24, 2026
Copilot AI requested a review from MichalStrehovsky August 24, 2026 01:02
@MichalStrehovsky
MichalStrehovsky marked this pull request as ready for review August 24, 2026 01:20
Copilot AI lite review requested due to automatic review settings August 24, 2026 01:20
@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.

@MichalStrehovsky

Copy link
Copy Markdown
Member

@dotnet/jit-contrib I'm running into the ImmedValNeedsReloc assert in #131526

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

Fixes xarch containment analysis for indirections so the “don’t contain relocatable constants for SIMD12 (Vector3)” restriction is applied to the indirection node type rather than the address operand type, ensuring NativeAOT/reloc scenarios don’t incorrectly contain a relocatable constant address.

Changes:

  • Update Lowering::ContainCheckIndir to gate the relocatable-constant containment rule on node->TypeIs(TYP_SIMD12) instead of addr->TypeIs(TYP_SIMD12).
  • Prevent SIMD12 indir codegen paths (which may adjust the address by +8) from encountering contained relocatable address constants on xarch.

@jakobbotsch
jakobbotsch merged commit 46d32bd into main Aug 24, 2026
132 checks passed
@jakobbotsch
jakobbotsch deleted the copilot/fix-simd12-containment-check branch August 24, 2026 12:02
@dotnet-milestone-bot dotnet-milestone-bot Bot added this to the 12.0-preview1 milestone Aug 25, 2026
@github-actions github-actions Bot locked and limited conversation to collaborators Sep 24, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

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.

5 participants