Move `Thread::m_pInterpThreadContext` near top of Thread class for stable asm offset by Copilot · Pull Request #126684 · dotnet/runtime

Copilot · 2026-04-09T02:20:01Z

Thread::m_pInterpThreadContext was declared at the very end of the Thread class, making its offset highly volatile across build configurations. Moving it towards the top of the Thread class to a small set of stable, always-present fields at the top of the Thread class will make it a lot more stable, and less prone to build breaks.

…thAllocContextObj This makes the offset more stable for asm code. The field is no longer at the end of the class (offsets were 0x290-0xaf8 depending on config), and is now at a stable offset of 0x30 (64-bit) or 0x1c (32-bit) near the top of the class. Update OFFSETOF__Thread__m_pInterpThreadContext in all asmconstants.h files (amd64, arm64, arm, loongarch64, riscv64) to the new stable values. The conditional compilation guards for _DEBUG and TARGET_UNIX are removed since the value is now the same in all configurations for each architecture. Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/21a58369-8e3e-4715-98ba-f11a627ecdfe Co-authored-by: jkotas <6668460+jkotas@users.noreply.github.com>

src/coreclr/vm/threads.h

…r near top of Thread class Move the entire #ifdef FEATURE_INTERPRETER block (public: access specifier, m_pInterpThreadContext field, and both method declarations) from the end of the Thread class to just after m_thAllocContextObj. This consolidates all interpreter-related declarations into a single #ifdef block near the top of the class, while keeping the stable offset for m_pInterpThreadContext. Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/b426d848-547f-405d-97e3-1e21bf94368d Co-authored-by: jkotas <6668460+jkotas@users.noreply.github.com>

dotnet-policy-service · 2026-04-09T04:29:18Z

Tagging subscribers to this area: @BrzVlad, @janvorli, @kg
See info in area-owners.md if you want to be subscribed.

Copilot

Pull request overview

This PR stabilizes the assembly-visible offset of Thread::m_pInterpThreadContext by moving the FEATURE_INTERPRETER field/method declarations near the top of Thread, avoiding large layout drift across OS and debug/release builds.

Changes:

Relocated the FEATURE_INTERPRETER block (field + access specifier + method declarations) to immediately follow m_thAllocContextObj in src/coreclr/vm/threads.h.
Updated architecture-specific OFFSETOF__Thread__m_pInterpThreadContext constants to match the new layout, removing per-OS/per-debug variants where no longer needed.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
src/coreclr/vm/threads.h	Moves the interpreter thread-context field/method declarations near the top of `Thread` to stabilize its offset.
src/coreclr/vm/amd64/asmconstants.h	Updates `OFFSETOF__Thread__m_pInterpThreadContext` to the new stable value and removes configuration conditionals.
src/coreclr/vm/arm64/asmconstants.h	Same offset update/conditional removal for arm64.
src/coreclr/vm/arm/asmconstants.h	Same offset update/conditional removal for arm (32-bit).
src/coreclr/vm/loongarch64/asmconstants.h	Same offset update for loongarch64.
src/coreclr/vm/riscv64/asmconstants.h	Same offset update for riscv64.

jkotas · 2026-04-09T04:34:25Z

This should fix build breaks in some of the runtime-extra-platforms legs: https://dev.azure.com/dnceng-public/public/_build/results?buildId=1370910&view=logs&jobId=fbbe6d24-ec94-5563-68e4-47df2fb4f886&j=fbbe6d24-ec94-5563-68e4-47df2fb4f886&t=fd24ffdb-b6c4-50de-7ee2-f1794d18b0a7

jkotas · 2026-04-09T04:35:05Z

/azp run runtime-extra-platforms

azure-pipelines · 2026-04-09T04:35:19Z

Azure Pipelines successfully started running 1 pipeline(s).

github-actions · 2026-04-09T04:43:57Z

🤖 Copilot Code Review — PR #126684

Note

This review was generated by Copilot.

Holistic Assessment

Motivation: Moving m_pInterpThreadContext near the top of the Thread class eliminates complex conditional-compilation offsets (_DEBUG/TARGET_UNIX combinations) in asmconstants.h files across 5 architectures. The problem is real — the old offsets ranged from 0x0 to 0xb50 depending on configuration, creating a maintenance burden and fragility for assembly code.

Approach: Relocating the field after m_thAllocContextObj gives it a stable offset (0x30 on 64-bit, 0x1c on 32-bit) that doesn't depend on config-specific fields that come later in the class. This is the right approach — it follows the existing pattern established by the hot fields at the top of Thread (lines 851–865 comment: "keep near the top of the object").

Summary: ✅ LGTM. Clean, well-scoped field relocation with correct offset updates across all architectures.

Detailed Findings

✅ Correctness — Offset safety verified by compile-time asserts

All asmconstants.h files retain ASMCONSTANTS_C_ASSERT(OFFSETOF__Thread__m_pInterpThreadContext == offsetof(Thread, m_pInterpThreadContext)), which will catch any offset mismatch at compile time. All 7 assembly sites that reference this constant (amd64 .S/.asm, arm .S, arm64 .S/.asm, loongarch64, riscv64) use the symbolic constant rather than hardcoded values.

✅ cDAC compatibility — No impact

cdac_data<Thread> (threads.h:3763) uses offsetof() for all its members and does not reference m_pInterpThreadContext, so the field relocation has no diagnostic impact.

✅ Layout impact — Minimal and safe

When FEATURE_INTERPRETER is defined, fields after m_thAllocContextObj are shifted by one pointer (8 bytes on 64-bit). The critical hot fields (m_State, m_fPreemptiveGCDisabled, m_pFrame, m_ThreadId) are all before the insertion point and unaffected. All downstream offsets are consumed through offsetof() or C++ member access, so they adjust automatically.

✅ Scope — Appropriately focused

The PR touches only the necessary files: threads.h for the field move, and 5 architecture-specific asmconstants.h files for the offset constants. No unrelated changes. x86 (i386) is correctly untouched since FEATURE_INTERPRETER is not supported there.

💡 Style — Redundant `public:` specifier

The public: at line 906 is technically redundant since the preceding context (from line 878) is already public. This is a non-issue — it matches the original code's style and improves clarity when reading the #ifdef block in isolation.

Generated by Code Review for issue #126684 · ◷

jkotas · 2026-04-09T20:51:43Z

/ba-g other extra platforms failures are being worked on

Copilot AI assigned Copilot and jkotas Apr 9, 2026

Copilot AI review requested due to automatic review settings April 9, 2026 02:20

Copilot created this pull request from a session on behalf of jkotas April 9, 2026 02:20 View session

Copilot AI review requested due to automatic review settings April 9, 2026 02:20

jkotas reviewed Apr 9, 2026

View reviewed changes

src/coreclr/vm/threads.h Outdated Show resolved Hide resolved

Copilot started work on behalf of jkotas April 9, 2026 02:20 View session

Copilot AI requested review from Copilot and jkotas and removed request for Copilot April 9, 2026 02:33

Copilot finished work on behalf of jkotas April 9, 2026 02:35

github-actions bot added the area-VM-coreclr label Apr 9, 2026

jkotas added area-CodeGen-Interpreter-coreclr and removed area-VM-coreclr labels Apr 9, 2026

jkotas marked this pull request as ready for review April 9, 2026 04:28

Copilot AI review requested due to automatic review settings April 9, 2026 04:28

jkotas approved these changes Apr 9, 2026

View reviewed changes

Copilot started reviewing on behalf of jkotas April 9, 2026 04:30 View session

Copilot AI reviewed Apr 9, 2026

View reviewed changes

Merge branch 'main' into copilot/move-m-pinterpthreadcontext

71827bb

jkotas requested a review from BrzVlad April 9, 2026 04:35

jkotas requested a review from janvorli April 9, 2026 04:35

BrzVlad approved these changes Apr 9, 2026

View reviewed changes

jkotas merged commit 797626d into main Apr 9, 2026
113 of 153 checks passed

jkotas deleted the copilot/move-m-pinterpthreadcontext branch April 9, 2026 20:52

dotnet-maestro bot mentioned this pull request Apr 9, 2026

[main] Source code updates from dotnet/runtime dotnet/dotnet#5961

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Move `Thread::m_pInterpThreadContext` near top of Thread class for stable asm offset#126684

Move `Thread::m_pInterpThreadContext` near top of Thread class for stable asm offset#126684
jkotas merged 3 commits intomainfrom
copilot/move-m-pinterpthreadcontext

Copilot AI commented Apr 9, 2026 •

edited by jkotas

Loading

Uh oh!

Uh oh!

dotnet-policy-service bot commented Apr 9, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

jkotas commented Apr 9, 2026

Uh oh!

jkotas commented Apr 9, 2026

Uh oh!

azure-pipelines bot commented Apr 9, 2026

Uh oh!

github-actions bot commented Apr 9, 2026

Uh oh!

jkotas commented Apr 9, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

Copilot AI commented Apr 9, 2026 • edited by jkotas Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

dotnet-policy-service bot commented Apr 9, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

jkotas commented Apr 9, 2026

Uh oh!

jkotas commented Apr 9, 2026

Uh oh!

azure-pipelines bot commented Apr 9, 2026

Uh oh!

github-actions bot commented Apr 9, 2026

🤖 Copilot Code Review — PR #126684

Holistic Assessment

Detailed Findings

✅ Correctness — Offset safety verified by compile-time asserts

✅ cDAC compatibility — No impact

✅ Layout impact — Minimal and safe

✅ Scope — Appropriately focused

💡 Style — Redundant public: specifier

Uh oh!

jkotas commented Apr 9, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Copilot AI commented Apr 9, 2026 •

edited by jkotas

Loading

💡 Style — Redundant `public:` specifier