Skip to content

P/Invokes backed by transient IL#126509

Open
jkoritzinsky wants to merge 11 commits intodotnet:mainfrom
jkoritzinsky:pinvoke-no-stub
Open

P/Invokes backed by transient IL#126509
jkoritzinsky wants to merge 11 commits intodotnet:mainfrom
jkoritzinsky:pinvoke-no-stub

Conversation

@jkoritzinsky
Copy link
Copy Markdown
Member

This PR changes non-varargs P/Invokes to be backed by transient IL instead of by an IL stub.

This change removes 1 DynamicMethodDesc per P/Invoke and removes some interpreter workarounds we had for portable entrypoints (skipping the frame between the P/Invoke and the IL stub for perf) and Swift (ensuring we propagate back the SwiftError register reliably).

This change does not enable Tiered Compilation for P/Invoke methods. They are still considered not tierable and not eligible for ReJIT.

It also reverts #124579 as that work is no longer necessary now that regular P/Invokes are no longer using IL stubs.

jkoritzinsky and others added 7 commits April 2, 2026 21:01
…ause construction

PInvokeStubLinker previously managed try-catch-finally blocks by
manually creating labels, computing offsets, and constructing
ILStubEHClause records. ILCodeStream already provides
BeginTryBlock/EndTryBlock/BeginCatchBlock/EndCatchBlock/
BeginFinallyBlock/EndFinallyBlock APIs that handle this automatically.

Changes:
- Move m_buildingEHClauses and m_finishedEHClauses from ILCodeStream to
  ILStubLinker so EH clauses can span multiple code streams (the PInvoke
  stub's try-finally begins on the Marshal stream and ends on Cleanup).
- Relax BeginHandler assertion to allow calling before EndTryBlock, since
  the finally handler may begin on a different stream than the try body.
- Convert PInvokeStubLinker::Begin to use BeginTryBlock.
- Convert SetCleanupNeeded to use BeginFinallyBlock instead of manually
  creating the cleanup finally begin label.
- Convert PInvokeStubLinker::End to use EndTryBlock/EndFinallyBlock
  instead of manually creating try-end and finally-end labels.
- Convert EmitExceptionHandler to use EndTryBlock/BeginCatchBlock/
  EndCatchBlock instead of manual label management.
- Replace manual EH clause construction (AppendEHClause, PopulateEHSect,
  GetCleanupFinallyOffsets) with ILStubLinker::GetNumEHClauses and
  WriteEHClauses.
- Add ILStubLinker::GetEHClause for extracting resolved clause info
  for logging and ETW.
- Remove PInvokeStubLinker::ClearCode override and manual EH label
  fields (m_pCleanupTryBeginLabel, m_pCleanupTryEndLabel,
  m_pCleanupFinallyBeginLabel, m_pCleanupFinallyEndLabel).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This is no longer necessary as non-vararg forward P/Invokes are now truly non-shared and don't go through the ILStubCache infrastructure at all.
@dotnet-policy-service
Copy link
Copy Markdown
Contributor

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

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR changes CoreCLR’s handling of non-varargs P/Invokes to be implemented via transient IL rather than IL stubs, reducing per-P/Invoke stub overhead and removing related interpreter/workaround paths. It also reworks IL stub EH-clause tracking to support try/handler spans across multiple ILCodeStreams, and updates various call sites to reflect the new P/Invoke execution model.

Changes:

  • Switch non-varargs P/Invokes to transient-IL-backed implementations and adjust prestub/code-prep paths accordingly.
  • Move EH clause tracking from ILCodeStream into ILStubLinker, enabling cross-stream try/finally and try/catch constructs.
  • Update IL stub resolver finalization to accept configurable JIT flags and simplify EH section emission/consumption.

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
src/coreclr/vm/stubgen.h Moves EH clause tracking ownership to ILStubLinker; adds GetEHClause() API.
src/coreclr/vm/stubgen.cpp Updates EH tracking usage across streams; adds resolved EH-clause extraction; clears EH arrays on ClearCode().
src/coreclr/vm/prestub.cpp Routes non-varargs P/Invokes through PrepareInitialCode + explicit P/Invoke target resolution; varargs still use stubs.
src/coreclr/vm/method.cpp Treats non-varargs P/Invokes as “may have native code”; explicitly excludes P/Invoke from tiered compilation eligibility; changes x86 stack arg sizing path.
src/coreclr/vm/jitinterface.cpp Treats P/Invokes like IL/dynamic methods for getMethodInfo when no IL header is present.
src/coreclr/vm/ilstubresolver.h Extends FinalizeILStub to take CORJIT_FLAGS (defaulting to CORJIT_FLAG_IL_STUB).
src/coreclr/vm/ilstubresolver.cpp Uses passed-in JIT flags instead of hardcoding IL-stub flags.
src/coreclr/vm/dllimport.h Adds APIs for transient-IL P/Invoke generation and for resolving targets; removes now-unused flag helpers.
src/coreclr/vm/dllimport.cpp Implements transient IL generation for P/Invoke; refactors EH handling and stub finalization; updates vararg P/Invoke stub flow.
src/coreclr/inc/CrstTypes.def Adjusts ILStubGen lock ordering metadata.

@jkotas
Copy link
Copy Markdown
Member

jkotas commented Apr 3, 2026

Does the change enable P/Invoke stub inlining? We may want to check the impact on JIT throughput.

@AaronRobinsonMSFT
Copy link
Copy Markdown
Member

AaronRobinsonMSFT commented Apr 3, 2026

Does the change enable P/Invoke stub inlining? We may want to check the impact on JIT throughput.

This is an important thing to check. When the transient IL logic was written we worked to make it "natural" so that from the JIT's perspective it has no clue where the IL comes from. This meant all the UnsafeAccessor functions just got inlined due to their size. Unless something has changed the inlining will happen using the same logic as any other IL JIT logic. This could then result in an issue with if checks not blocking missing DLL exceptions for platform checks.

This is a long way of saying, we need to seriously think about making all of these inlinable as it could be a very distruptive change. I'm not sure which side I'd come down on now since we have LibraryImport.

@jkotas
Copy link
Copy Markdown
Member

jkotas commented Apr 3, 2026

This could then result in an issue with if checks not blocking missing DLL exceptions for platform checks.

The system is setup to resolve these lazily only once they are actually called, even in the presence of inlining. This should not be a problem (modulo bugs).

@AaronRobinsonMSFT
Copy link
Copy Markdown
Member

This could then result in an issue with if checks not blocking missing DLL exceptions for platform checks.

The system is setup to resolve these lazily only once they are actually called, even in the presence of inlining. This should not be a problem (modulo bugs).

Ugh. Why do I keep making this bad assumption. I need a brick to the head or something.

…lists

Replace SArray<ILStubEHClauseBuilder> with intrusive singly linked lists
for both the building (stack) and finished (queue) EH clause collections.

- m_buildingEHClauses becomes m_pBuildingEHClauseStack (head pointer)
- m_finishedEHClauses becomes m_pFinishedEHClauseHead/Tail pointers
- EndHandler moves nodes between lists (zero-copy) instead of copying
- Add DeleteEHClauses() for cleanup in destructor and ClearCode

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings April 6, 2026 21:36
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

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 11 out of 11 changed files in this pull request and generated 1 comment.

@jkoritzinsky
Copy link
Copy Markdown
Member Author

/azp run runtime,runtime-dev-innerloop,dotnet-linker-tests

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 3 pipeline(s).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

4 participants