Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

JIT: Unify InlinedCallFrame secret stub arg handling #100662

Closed
jakobbotsch opened this issue Apr 5, 2024 · 1 comment · Fixed by #100823
Closed

JIT: Unify InlinedCallFrame secret stub arg handling #100662

jakobbotsch opened this issue Apr 5, 2024 · 1 comment · Fixed by #100823
Labels
area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI
Milestone

Comments

@jakobbotsch
Copy link
Member

For some IL stubs with pinvokes in them the runtime passes a non-standard argument that the JIT must save in a known place. Currently this mechanism is different between 32 bit and 64 bit platforms. On 32 bit platforms the argument is stored by the JIT in the local stack frame at a known place:

// Given a methodDesc representing an ILStub for a pinvoke call,
// this method will return the MethodDesc for the actual interop
// method if the current InlinedCallFrame is inactive.
PTR_MethodDesc GetActualInteropMethodDesc()
{
#if defined(TARGET_X86) || defined(TARGET_ARM)
// Important: This code relies on the way JIT lays out frames. Keep it in sync
// with code:Compiler.lvaAssignFrameOffsets.
//
// | ... |
// +--------------------+
// | lvaStubArgumentVar | <= filled with EAX in prolog |
// +--------------------+ |
// | | |
// | InlinedCallFrame | |
// | | <= m_pCrawl.pFrame | to lower addresses
// +--------------------+ V
// | ... |
//
// Extract the actual MethodDesc to report from the InlinedCallFrame.
TADDR addr = dac_cast<TADDR>(this) + sizeof(InlinedCallFrame);
return PTR_MethodDesc(*PTR_TADDR(addr));
#elif defined(HOST_64BIT)
// On 64bit, the actual interop MethodDesc is saved off in a field off the InlinedCrawlFrame
// which is populated by the JIT. Refer to JIT_InitPInvokeFrame for details.
return PTR_MethodDesc(m_StubSecretArg);
#else
_ASSERTE(!"NYI - Interop method reporting for this architecture!");
return NULL;
#endif // defined(TARGET_X86) || defined(TARGET_ARM)
}

On 64-bit platforms the argument is instead passed to the CORINFO_HELP_INIT_PINVOKE_FRAME helper call.

We can unify these mechanisms in the way suggested by @jkotas here, by storing it directly inside the InlinedFrame that the JIT already has special knowledge of. It will save us some special casing within the JIT and avoid the unnecessary store on 64-bit in many cases.

@dotnet-issue-labeler dotnet-issue-labeler bot added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Apr 5, 2024
@jakobbotsch jakobbotsch added this to the Future milestone Apr 5, 2024
Copy link
Contributor

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

@jakobbotsch jakobbotsch changed the title JIT: Unify InlinedFrame secret stub arg handling JIT: Unify InlinedCallFrame secret stub arg handling Apr 5, 2024
jakobbotsch added a commit to jakobbotsch/runtime that referenced this issue Apr 9, 2024
jakobbotsch added a commit to jakobbotsch/runtime that referenced this issue Apr 9, 2024
Ruihan-Yin pushed a commit to Ruihan-Yin/runtime that referenced this issue May 30, 2024
When a pinvoke/reverse pinvoke needs marshalling the VM creates an IL stub to
perform this marshalling. These IL stubs have a non-standard parameter called
the "secret stub parameter". The VM uses this parameter to map the IL stub back
to the user defined function that caused the IL stub to be required (multiple
user functions can share the same IL stubs, so the mapping cannot be done by
other means).

To facilitate the access of this value the JIT must make the parameter's value
available to the VM somehow. Previously this was done in two separate ways for
32-bit and 64-bit target:
- For 32-bit targets the parameter was marked as do-not-enregister and always
  spilled to the stack frame at a location that was known by the VM.
- For 64-bit targets the parameter was saved in the `InlinedCallFrame` as part
  of a VM helper call. We still marked it as do-not-enregister, probably because
  parameter homing did not handle it.

For 64-bit targets this introduces a bit of inefficiency: the secret stub
parameter is only needed for the IL stubs case, but `InlinedCallFrame` is used
for all inlined pinvokes. So we ended up with a larger frame than necessary and
with an additional store to the frame, even outside IL stubs.

This change removes that inefficiency by unifying how 32-bit and 64-bit targets
work:

- Switch all platforms to only allocate space in `InlinedCallFrame` for the
  secret stub parameter when necessary
- Move responsibility of storing the secret stub parameter out of
  `CORINFO_HELP_INIT_PINVOKE_FRAME` and to the JIT generated code
- Remove special casing of frame layout around the secret stub parameter and
  frame structures within the JIT
- Enable enregistration for the secret stub parameter

Fix dotnet#100662
@github-actions github-actions bot locked and limited conversation to collaborators Jun 12, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant