Skip to content

JIT: stop treating ManagedThreadId helper as constant/pure - #130006

Merged
VSadov merged 7 commits into
mainfrom
copilot/remove-unnecessary-threadid-constant
Jul 2, 2026
Merged

JIT: stop treating ManagedThreadId helper as constant/pure#130006
VSadov merged 7 commits into
mainfrom
copilot/remove-unnecessary-threadid-constant

Conversation

CopilotAI commented Jun 29, 2026

Copy link
Copy Markdown
Contributor

This PR removes JIT assumptions that can treat CORINFO_HELP_GETCURRENTMANAGEDTHREADID as stable across async suspension, where execution may resume on a different thread.

  • Description

    • Removed CORINFO_HELP_GETCURRENTMANAGEDTHREADID -> VNF_ManagedThreadId from fgValueNumberJitHelperMethodVNFunc in src/coreclr/jit/valuenum.cpp.
    • Removed the now-unused ValueNumFuncDef(ManagedThreadId, 0, false, false) from src/coreclr/jit/valuenumfuncs.h.
    • Kept a narrow fallback in fgValueNumberHelperCall to treat CORINFO_HELP_GETCURRENTMANAGEDTHREADID as non-pure for VN purposes if it survives as a helper call, so it goes through opaque VN handling instead of function-mapped VN.
    • Updated HelperCallProperties::init() in src/coreclr/jit/utils.cpp so CORINFO_HELP_GETCURRENTMANAGEDTHREADID is no longer marked pure, while still marked no-throw.
  • What stays unchanged

    • Helper identity and JIT/EE interface are unchanged (CORINFO_HELP_GETCURRENTMANAGEDTHREADID remains).
    • Inline-candidate conversion path (impConvertToUserCallAndMarkForInlining) is unchanged.
    • The helper remains no-throw.
// Removed from fgValueNumberJitHelperMethodVNFunc:caseCORINFO_HELP_GETCURRENTMANAGEDTHREADID:
vnf = VNF_ManagedThreadId;
break;

@dotnet-policy-service

Copy link
Copy Markdown
Contributor

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

CopilotAI requested review from Copilot and removed request for CopilotJune 29, 2026 22:52
CopilotAI changed the title [WIP] Remove unnecessary constant for managed thread ID helperJIT: remove ManagedThreadId constant VN mapping to avoid cross-await reuseJun 29, 2026
CopilotAI requested a review from VSadovJune 29, 2026 23:02
@VSadov

Copy link
Copy Markdown
Member

@copilot we should also not mark the helper as "pure" in general, so that it cannot be moved and cross async suspend points. It is still valid to treat it as no-throw.

CopilotAI commented Jun 30, 2026

Copy link
Copy Markdown
ContributorAuthor

@copilot we should also not mark the helper as "pure" in general, so that it cannot be moved and cross async suspend points. It is still valid to treat it as no-throw.

Updated in da58ef9: CORINFO_HELP_GETCURRENTMANAGEDTHREADID is no longer marked pure in HelperCallProperties::init(), and it remains marked no-throw (exceptions = ExceptionSetFlags::None).

CopilotAI changed the title JIT: remove ManagedThreadId constant VN mapping to avoid cross-await reuseJIT: stop treating ManagedThreadId helper as constant/pure across async suspensionJun 30, 2026
@VSadovVSadov changed the title JIT: stop treating ManagedThreadId helper as constant/pure across async suspensionJIT: stop treating ManagedThreadId helper as constant/pureJun 30, 2026
@VSadov
VSadov marked this pull request as ready for review June 30, 2026 05:21
CopilotAI review requested due to automatic review settings June 30, 2026 05:21

CopilotAI 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

This change updates CoreCLR JIT value numbering and helper metadata so CORINFO_HELP_GETCURRENTMANAGEDTHREADID is no longer modeled as a pure / method-constant operation, preventing VN from treating repeated uses as equivalent across the method.

Changes:

  • Removed the VN function (VNF_ManagedThreadId) plumbing for CORINFO_HELP_GETCURRENTMANAGEDTHREADID.
  • Updated helper call properties so CORINFO_HELP_GETCURRENTMANAGEDTHREADID is no longer marked pure (while remaining no-throw).
  • Added a VN-path guard to ensure the helper is treated as non-pure during helper-call VN handling.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

FileDescription
src/coreclr/jit/valuenumfuncs.hRemoves the VNFunc definition previously used for ManagedThreadId.
src/coreclr/jit/valuenum.cppStops mapping the helper to a VNFunc and adjusts helper-call VN handling accordingly.
src/coreclr/jit/utils.cppUpdates helper metadata so GETCURRENTMANAGEDTHREADID is no longer classified as pure.

Comment threadsrc/coreclr/jit/valuenum.cpp Outdated
CopilotAIand others added 6 commits July 2, 2026 13:26
Co-authored-by: VSadov <8218165+VSadov@users.noreply.github.com>
Co-authored-by: VSadov <8218165+VSadov@users.noreply.github.com>
Co-authored-by: VSadov <8218165+VSadov@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: VSadov <8218165+VSadov@users.noreply.github.com>
Co-authored-by: VSadov <8218165+VSadov@users.noreply.github.com>
CopilotAI review requested due to automatic review settings July 2, 2026 20:26
@VSadov
VSadovforce-pushed the copilot/remove-unnecessary-threadid-constant branch from 0ca64b3 to cc701c2CompareJuly 2, 2026 20:26

CopilotAI 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

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

Comment threadsrc/coreclr/jit/utils.cpp
…sion
Co-authored-by: VSadov <8218165+VSadov@users.noreply.github.com>

CopilotAI commented Jul 2, 2026

Copy link
Copy Markdown
ContributorAuthor

@copilot we should also not mark the helper as "pure" in general, so that it cannot be moved and cross async suspend points. It is still valid to treat it as no-throw.

Added in the latest commit (src/tests/async/regression/managed-thread-id.cs):

  1. TestThreadIdMatchesCurrentThread — verifies Environment.CurrentManagedThreadId == Thread.CurrentThread.ManagedThreadId both before and after await Task.Delay(1).

  2. TestThreadIdReflectsResumptionThread — uses a custom awaiter (SwitchToNewThread) that guarantees the continuation runs on a brand-new dedicated thread, then asserts the thread ID reflects the resumption thread and differs from the pre-await thread ID.

The custom awaiter pattern matches the existing YieldToThreadWithCustomSyncContext approach in synchronization-context.cs, ensuring the thread switch is deterministic rather than relying on TCS inline-completion behavior.

@VSadov

Copy link
Copy Markdown
Member

Thanks!

@VSadov
VSadov enabled auto-merge (squash) July 2, 2026 22:18
@VSadov
VSadov merged commit fe5e473 into mainJul 2, 2026
140 of 143 checks passed
@VSadov
VSadov deleted the copilot/remove-unnecessary-threadid-constant branch July 2, 2026 23:21
@dotnet-milestone-botdotnet-milestone-botBot added this to the 11.0-preview7 milestone Jul 3, 2026
eiriktsarpalis pushed a commit that referenced this pull request Jul 15, 2026
This PR removes JIT assumptions that can treat
`CORINFO_HELP_GETCURRENTMANAGEDTHREADID` as stable across async
suspension, where execution may resume on a different thread.
- **Description**
- Removed `CORINFO_HELP_GETCURRENTMANAGEDTHREADID ->
VNF_ManagedThreadId` from `fgValueNumberJitHelperMethodVNFunc` in
`src/coreclr/jit/valuenum.cpp`.
- Removed the now-unused `ValueNumFuncDef(ManagedThreadId, 0, false,
false)` from `src/coreclr/jit/valuenumfuncs.h`.
- Kept a narrow fallback in `fgValueNumberHelperCall` to treat
`CORINFO_HELP_GETCURRENTMANAGEDTHREADID` as non-pure for VN purposes if
it survives as a helper call, so it goes through opaque VN handling
instead of function-mapped VN.
- Updated `HelperCallProperties::init()` in `src/coreclr/jit/utils.cpp`
so `CORINFO_HELP_GETCURRENTMANAGEDTHREADID` is no longer marked pure,
while still marked no-throw.
- **What stays unchanged**
- Helper identity and JIT/EE interface are unchanged
(`CORINFO_HELP_GETCURRENTMANAGEDTHREADID` remains).
- Inline-candidate conversion path
(`impConvertToUserCallAndMarkForInlining`) is unchanged.
- The helper remains no-throw.
```cpp
// Removed from fgValueNumberJitHelperMethodVNFunc:
case CORINFO_HELP_GETCURRENTMANAGEDTHREADID:
vnf = VNF_ManagedThreadId;
break;
```
---------
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: VSadov <8218165+VSadov@users.noreply.github.com>
Co-authored-by: Vladimir Sadov <vsadov@microsoft.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
@github-actionsgithub-actionsBot locked and limited conversation to collaborators Aug 3, 2026
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@VSadov@jakobbotsch