Include the thumb bit in symbol definitions for method code nodes in crossgen2#128235
Open
jtschuster wants to merge 4 commits into
Open
Include the thumb bit in symbol definitions for method code nodes in crossgen2#128235jtschuster wants to merge 4 commits into
jtschuster wants to merge 4 commits into
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Proof-of-concept that makes crossgen2 emit ARM method symbols with the thumb bit already set (matching ILC/runtime behavior), removing the workarounds where the JIT and R2R nodes had to add the thumb bit (CodeDelta) on a per-relocation basis. Tables that store RVAs of methods (DelayLoadMethodCallThunks, ExceptionInfo, RuntimeFunctions) now compensate by subtracting CodeDelta since they don't want the thumb bit.
Changes:
- JIT (
emitarm.cpp,emit.cpp): drop ARM thumb-bit/R2R special cases formovw/movtand async resumption stubs; rely oncompRelocand on symbols already carrying the thumb bit. - R2R object writer nodes: stop adding
factory.Target.CodeDeltainDelayLoadHelperImport,MethodGCInfoNode(personality),ResumptionStubEntryPointSignature, andRuntimeFunctionsTableNode; subtract it inExceptionInfoLookupTableNodeto keep RVAs thumb-bit-free. ObjectWriter.cs: remove the!READYTORUNguard so crossgen2 also sets the thumb bit on ARM method symbols.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/coreclr/tools/Common/Compiler/ObjectWriter/ObjectWriter.cs | Always set thumb bit on ARM method symbols, including for R2R. |
| src/coreclr/tools/aot/ILCompiler.ReadyToRun/.../RuntimeFunctionsTableNode.cs | Remove CodeDelta from runtime-function RVA reloc. |
| src/coreclr/tools/aot/ILCompiler.ReadyToRun/.../ResumptionStubEntryPointSignature.cs | Drop CodeDelta from resumption stub entry-point reloc. |
| src/coreclr/tools/aot/ILCompiler.ReadyToRun/.../MethodGCInfoNode.cs | Drop CodeDelta on personality routine reloc; whitespace cleanups. |
| src/coreclr/tools/aot/ILCompiler.ReadyToRun/.../ExceptionInfoLookupTableNode.cs | Subtract CodeDelta to keep EH method entries as RVAs without thumb bit. |
| src/coreclr/tools/aot/ILCompiler.ReadyToRun/.../DelayLoadHelperImport.cs | Drop CodeDelta from delay-load helper reloc; whitespace cleanups. |
| src/coreclr/jit/emitarm.cpp | Use compReloc instead of IsNativeAot() to decide whether to add thumb bit to movw/movt literal. |
| src/coreclr/jit/emit.cpp | Remove R2R-specific +1 addend workaround for async resume stub relocs. |
This was referenced May 15, 2026
Open
jtschuster
changed the title
jtschuster
changed the title
Member
Author
|
/azp run runtime-coreclr |
|
No pipelines are associated with this pull request. |
Member
Author
|
/azp run runtime-coreclr crossgen2 |
|
Azure Pipelines successfully started running 1 pipeline(s). |
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
| int entries = 0; | ||
| for (int index = 0; index < count; index++, offset += runtimeFunctionSize) | ||
| { | ||
| int startRva = BitConverter.ToInt32(reader.Image, offset); |
Member
There was a problem hiding this comment.
BinaryPrimitives.ReadBlahBlah is better practice than BitConverter.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Member
Author
|
Full CI validation is dependent on #128170. However, the tests in ILCompiler.ReadyToRun.Tests give us some confidence that the changes are correct. |
davidwrighton
approved these changes
May 19, 2026
Member
davidwrighton
left a comment
davidwrighton
left a comment
There was a problem hiding this comment.
Approved modulo getting a successful validation run.
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.
Example of what code would look like if crossgen2 emitted the thumb bit into method symbols.
The differences in output are in the DelayLoadMethodCallThunks table and ExceptionInfo. Both of these currently have fields that are RVA's of methods, not
PCode, and are not used to jump into the method code, so they don't have the thumb bit. If we make this change, we'd either need to update the name and expectation, and maybe usePCodeinstead ofPInstrfor the fields, or create a new node type to represent the RVA of the method code rather that a pointer to the method code. I don't yet have a preference or intuition on what would be better.