SPMI: Fix relocs into new RO data section#124383
Merged
jakobbotsch merged 3 commits intodotnet:mainfrom Feb 19, 2026
Merged
Conversation
- Add arm64 handling similar to existing handling for relative relocs pointing into RO data section - Teach the relocation handling that on arm64 there are multiple RO data blocks (one at the end of code, one separate one for async functions)
github-actions
bot
added
the
area-CodeGen-coreclr
Contributor
|
Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates SuperPMI’s near-diff relocation normalization to better handle ARM64 relocations that reference a second RO-data block (not the constant pool appended to hot code), addressing spurious diffs seen for async-related data on linux-arm64.
Changes:
- Extend
RelocContextto track two RO-data ranges (primary + secondary) and thread that through near-diff relocation application. - Add ARM64-specific relocation handling intended to remap targets that land in the secondary RO-data block.
- Minor assignment reordering in
Compiler::impResolveToken.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/coreclr/tools/superpmi/superpmi/neardiffer.cpp | Tracks two RO-data regions on ARM64 and populates the expanded RelocContext accordingly. |
| src/coreclr/tools/superpmi/superpmi-shared/compileresult.h | Expands RelocContext with roDataSize1/2 and originalRoDataAddress1/2. |
| src/coreclr/tools/superpmi/superpmi-shared/compileresult.cpp | Adds ARM64 relocation remapping logic for targets that fall into the secondary RO-data range; updates uses to new context fields. |
| src/coreclr/jit/importer.cpp | Reorders token field assignments in impResolveToken. |
Comments suppressed due to low confidence (1)
src/coreclr/tools/superpmi/superpmi-shared/compileresult.cpp:890
- In the ARM64_PAGEBASE_REL21 relocation handling,
pageDeltais computed astargetPage - targetPage, which is always 0 and leavesfixupLocationPageunused. This should be computed relative to the fixup location page (e.g.,targetPage - fixupLocationPage) so ADRP immediates are patched correctly and reflect the recorded relocation target.
INT64 targetPage = (INT64)target & 0xFFFFFFFFFFFFF000LL;
INT64 fixupLocationPage = (INT64)fixupLocation & 0xFFFFFFFFFFFFF000LL;
INT64 pageDelta = (INT64)(targetPage - targetPage);
INT32 imm21 = (INT32)(pageDelta >> 12) & 0x1FFFFF;
PutArm64Rel21((UINT32*)address, imm21);
This was referenced Feb 13, 2026
Open
Member
Author
|
/azp run runtime-coreclr superpmi-diffs |
|
Azure Pipelines successfully started running 1 pipeline(s). |
Member
Author
|
PTAL @dotnet/jit-contrib. Fixes some spurious diffs. |
AndyAyersMS
approved these changes
Feb 19, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fix #124371
This is quite hacky. I opened #124382 to clean this up in the future.