Skip to content

Conversation

@nattress
Copy link
Contributor

@nattress nattress commented Feb 2, 2021

  • Crossgen2 presently abuses the CORINFO_SIG_INFO.pSig field to pass a handle representing a managed method signature object which the JIT passes back to us later in compilation. This field is for byte arrays and causes problems with SuperPMI which rightly doesn't know how to interpret a zero-length buffer whose pointer is set.
  • Introduce a new field ,methodSignature where we pass the handle back to the JIT. This pattern saves us from allocating and pinning a byte buffer for signatures, something the native code VM and Crossgen1 don't have to worry about.

@BruceForstall This should simplify the hackery needed to make SuperPMI work with CG2

@nattress
Copy link
Contributor Author

nattress commented Feb 2, 2021

cc @dotnet/crossgen-contrib

Copy link
Member

@davidwrighton davidwrighton left a comment

Choose a reason for hiding this comment

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

Yes, this looks good

Copy link
Contributor

@BruceForstall BruceForstall left a comment

Choose a reason for hiding this comment

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

LGTM.

I'll submit a PR to this branch with the SuperPMI side of the change. (You need to fix the merge conflict from my change to jiteeversionguid.h)

Copy link
Member

@MichalStrehovsky MichalStrehovsky left a comment

Choose a reason for hiding this comment

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

Nice, thank you!

* Crossgen2 presently abuses the `CORINFO_SIG_INFO` pSig field to pass a handle representing a managed method signature object which the JIT passes back to us later in compilation. This field is for byte arrays and causes problems with SuperPMI which rightly doesn't know how to interpret a zero-length buffer whose pointer is set.
* Introduce a new field ,`methodSignature` where we pass the handle back to the JIT. This pattern saves us from allocating and pinning a byte buffer for signatures, something the native code VM and Crossgen1 don't have to worry about.
* Add strongly typed `ObjectToHandle` and `HandleToObject` methods for `CORINFO_MODULE_STRUCT_` <=> `MethodIL` and `MethodSignatureInfo` <=> `MethodSignature`.
@nattress nattress force-pushed the spmi_signature_handle branch from e17f880 to 8e4da9e Compare February 3, 2021 20:41
@nattress nattress force-pushed the spmi_signature_handle branch from 76e3f7f to 0880c18 Compare February 3, 2021 21:08
@nattress nattress merged commit ba63deb into dotnet:master Feb 4, 2021
@nattress nattress deleted the spmi_signature_handle branch February 4, 2021 00:56
Copy link
Member

@trylek trylek left a comment

Choose a reason for hiding this comment

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

The removed typecasts look great, thanks for the wonderful cleanup!

@ghost ghost locked as resolved and limited conversation to collaborators Mar 6, 2021
Sign up for free to 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.

6 participants