Skip to content
This repository was archived by the owner on Jan 23, 2023. It is now read-only.

Conversation

pgavlin
Copy link

@pgavlin pgavlin commented Feb 5, 2016

This change adds support for CoreRT-style P/Invoke transitions
to RyuJIT. Instead of the usual inlined transition code, these
transitions are made up of calls to two new JIT helpers:

PInvokeTransitionFrame frame; // opaque local
CORINFO_HELP_INIT_PINVOKE_FRAME(&frame);
...
CORINFO_HELP_JIT_PINVOKE_BEGIN(&frame);
target(...);
CORINFO_HELP_JIT_PINVOKE_END(&frame);
...

The preemptive mode constraints apply between calls to
JIT_PINVOKE_BEGIN and JIT_PINVOKE_END: no managed references
may be live only in registers and managed references may not
be manipulated.

Note that this is a rebased version of the code from #2817.

This change adds support for CoreRT-style P/Invoke transitions
to RyuJIT. Instead of the usual inlined transition code, these
transitions are made up of calls to two new JIT helpers:

    PInvokeTransitionFrame frame; // opaque local
    CORINFO_HELP_INIT_PINVOKE_FRAME(&frame);
    ...
    CORINFO_HELP_JIT_PINVOKE_BEGIN(&frame);
    target(...);
    CORINFO_HELP_JIT_PINVOKE_END(&frame);
    ...

The preemptive mode constraints apply between calls to
JIT_PINVOKE_BEGIN and JIT_PINVOKE_END: no managed references
may be live only in registers and managed references may not
be manipulated.
@pgavlin
Copy link
Author

pgavlin commented Feb 5, 2016

@jkotas @dotnet/jit-contrib PTAL

@pgavlin
Copy link
Author

pgavlin commented Feb 5, 2016

@BruceForstall PTAL

src/inc/corjit.h Outdated
CORJIT_FLG_NO_MDIL = 0x00010000, // Generate an MDIL section but no code or CTL. Not used by the JIT, used internally by NGen only.
#else // MDIL
CORJIT_FLG_CFI_UNWIND = 0x00004000, // Emit CFI unwind info
#endif // MDIL

Choose a reason for hiding this comment

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

Why did you change this? It was intentional as it was, to keep identical bit patterns together.

Copy link
Author

Choose a reason for hiding this comment

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

Why did you change this?

This was part of the earlier change (which had FLG_USE_PINVOKE_HELPERS instead of FLG_CFI_UNWIND); I kept it when I rebased.

It was intentional as it was, to keep identical bit patterns together.

If we believe that keeping identical bit patterns together is important, then we should apply that to the rest of the file (e.g. the definitions for CORJIT_FLG_MAKEFINALCODE and CORJIT_FLG_READYTORUN should be next to CORJIT_FLG_MINIMAL_MDIL and CORJIT_FLG_NO_MDIL, respectively). I'm fine with either keeping all of the bits that are affected by an particular symbol together or keeping the bits themselves together. I have a minor preference for the former since it makes it easier to understand what changes when a given symbol is defined. Note that we already use that style for processor-specific preprocessor symbols.

@BruceForstall
Copy link

It would be nice to see a comment somewhere in the code (maybe by genPInvokeProlog(?)) that describes what the p/invoke function characteristics are (i.e., what the function looks like) with the new model, with examples.

@pgavlin
Copy link
Author

pgavlin commented Feb 5, 2016

i.e., what the function looks like

What characteristics are you looking for? Calling convention, signature, etc.?

@pgavlin
Copy link
Author

pgavlin commented Feb 5, 2016

Or just the shape of the generated code?

@BruceForstall
Copy link

all of the above

@pgavlin
Copy link
Author

pgavlin commented Feb 5, 2016

Feedback has been addressed; PTAL.

@BruceForstall
Copy link

LGTM

@pgavlin
Copy link
Author

pgavlin commented Feb 6, 2016

@dotnet-bot test this please

pgavlin added a commit that referenced this pull request Feb 6, 2016
Generate P/Invoke transitions for CoreRT.
@pgavlin pgavlin merged commit 9c6aaa5 into dotnet:master Feb 6, 2016
@pgavlin pgavlin deleted the CoreRTPInvoke branch February 6, 2016 02:25
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants