Arm64: don't fold ldr page offset (PAGEOFFSET_12L) for NativeAOT#129966
Conversation
The adrp+add+ldr to adrp+ldr[#:lo12:] fold uses a 64-bit PAGEOFFSET_12L reloc (R_AARCH64_LDST64_ABS_LO12_NC), whose scaled offset requires the target to be 8-byte aligned. R2R loads always go through pointer-aligned indirection cells, but NativeAOT can fold a direct 64-bit load of byte-packed frozen data (4-byte aligned), which the linker rejects with 'improper alignment for relocation'. Restrict the fold to non-NativeAOT. Fixes dotnet#129936 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Tagging subscribers to this area: @agocke, @dotnet/ilc-contrib |
There was a problem hiding this comment.
Pull request overview
This PR adjusts the Arm64 emitter’s TryFoldPageOffsetIntoLdr peephole optimization to avoid emitting PAGEOFFSET_12L relocations when targeting the NativeAOT ABI, preventing generation of linker-invalid R_AARCH64_LDST64_ABS_LO12_NC relocations in scenarios where the referenced symbol may not be 8-byte aligned.
Changes:
- Disable
adrp+add+ldr→adrp+ldr[#:lo12:]folding for NativeAOT by early-returning inTryFoldPageOffsetIntoLdr. - Add an in-code rationale describing the alignment requirement of
PAGEOFFSET_12L/R_AARCH64_LDST64_ABS_LO12_NCand why NativeAOT differs.
Co-authored-by: Jakob Botsch Nielsen <Jakob.botsch.nielsen@gmail.com>
The adrp+add+ldr -> adrp+ldr[#:lo12:] fold uses R_AARCH64_LDST64_ABS_LO12_NC, which encodes the :lo12: page offset scaled by 8, so the reloc target must be 8-byte aligned. Replace the blanket NativeAOT opt-out with a new JIT-EE API, getAddressAlignment(void*), and only fold when the VM guarantees the target is at least 8-byte aligned. - VM (CoreCLR/NGen): reports the alignment of the absolute address. - crossgen2 (R2R): reports pointer size; direct relocatable loads only target pointer-aligned data. - NativeAOT: reports the exact alignment of non-GC statics (the failing case) and pointer size for GC-statics/RVA data, and fails closed otherwise so an unaligned relocation can never be emitted. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
MichalStrehovsky
left a comment
There was a problem hiding this comment.
Thank you for making it work for native AOT!
…Alignment Replace the duplicated per-node alignment logic in getAddressAlignment with a new IObjectNodeWithAlignment interface implemented by NonGCStaticsNode, GCStaticsNode and FieldRvaDataNode. Each node's GetData/GetDehydratableData now uses GetAlignment(factory) so the value reported to the JIT can never drift from the alignment actually baked into the node's data. Also move getAddressAlignment out of the shared CorInfoImpl.cs (dropping the #if READYTORUN split) into CorInfoImpl.ReadyToRun.cs and CorInfoImpl.RyuJit.cs. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…tion) Address review feedback: instead of R2R's blanket `return PointerSize`, resolve the reloc target node and report its real alignment, mirroring the NativeAOT implementation. Adds a parallel R2R IObjectNodeWithAlignment interface (R2R has a separate node/NodeFactory system, so it can't share the NAOT one) implemented on Import (returns the pointer-sized import-section EntrySize) and CopiedFieldRvaNode. Unknown targets fall back to 1 so the JIT never emits an alignment-sensitive reloc it can't prove is legal. Validated: full System.Private.CoreLib arm64 R2R crossgen compiles cleanly; every LDST64 fold candidate is an Import subclass reporting align=8 (no MISS, no align<8), so the adrp+add+ldr -> adrp+ldr[:lo12:] fold is preserved. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The R2R and NAOT bodies are now identical, so host it once in the shared partial. Each compiler still binds its own IObjectNodeWithAlignment and NodeFactory (only one is visible per compilation), so it resolves correctly. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…naot # Conflicts: # src/coreclr/inc/jiteeversionguid.h
|
@jkotas @MichalStrehovsky anything else here? |
MichalStrehovsky
left a comment
There was a problem hiding this comment.
We now have two IObjectNodeWithAlignment.cs files with slightly different comments. But I really just want to unblock the CI. I would have preferred we revert the change that broke ARM64 testing the moment we found out it's not quite right and then do it properly at leisure.
|
RyuJIT assert == build break and we cannot test anything. |
Fixes #129936.
PR #129589 added an Arm64 fold of
adrp+add+ldrintoadrp+ldr[#:lo12:]via a 64-bitPAGEOFFSET_12Lreloc (R_AARCH64_LDST64_ABS_LO12_NC). That reloc encodes the offset scaled by 8, so the target must be 8-byte aligned. In R2R the relocatable loads always go through pointer-aligned indirection cells, but NativeAOT can fold a direct 64-bit load of byte-packed frozen data (4-byte aligned), whichld.lldrejects:The fold is purely a size optimization, so this restricts it to non-NativeAOT and falls back to the always-safe
adrp+add+ldr.Validation (SPMI, linux-arm64):
benchmarks.run(R2R, ~427K contexts): 0 asm diffs — the win is preserved.smoke_tests.nativeaot(18891 ctx): clean replay, no diffs.