Skip to content

Conversation

@teo-tsirpanis
Copy link
Contributor

I noticed that the ILGenerator class in Mono uses an object stack to store integers, boxing them for no reason, and that some code that emits numbers in the code stream can be simplified with the help of members of the BinaryPrimitives class. This PR fixes both things.

… stream.

An allocation is saved when emitting instructions with floating-point arguments.
@ghost ghost added community-contribution Indicates that the PR has been added by a community member area-System.Reflection.Emit labels Feb 12, 2022
@ghost
Copy link

ghost commented Feb 12, 2022

Tagging subscribers to this area: @dotnet/area-system-reflection-emit
See info in area-owners.md if you want to be subscribed.

Issue Details

I noticed that the ILGenerator class in Mono uses an object stack to store integers, boxing them for no reason, and that some code that emits numbers in the code stream can be simplified with the help of members of the BinaryPrimitives class. This PR fixes both things.

Author: teo-tsirpanis
Assignees: -
Labels:

area-System.Reflection.Emit, community-contribution

Milestone: -

@stephentoub
Copy link
Member

How much work would it be to instead unify on coreclr's ILGenerator, moving that to shared and only specializing for runtime where there's a real need for divergence?

@teo-tsirpanis
Copy link
Contributor Author

That's a nice idea but not something at least I intend to do any time soon. Better open a new issue to not forget it.

@lambdageek lambdageek added area-VM-reflection-mono Reflection issues specific to MonoVM and removed area-System.Reflection.Emit labels Feb 12, 2022
@lambdageek
Copy link
Member

lambdageek commented Feb 12, 2022

How much work would it be to instead unify on coreclr's ILGenerator, moving that to shared and only specializing for runtime where there's a real need for divergence?

it's probably not too much work. The CoreCLR implementation uses SignatureHelper.GetLocalVarSigHelper (Module? module) which is ok. The place where things get tricky is in code like DynamicILInfo that uses SignatureHelper.GetLocalVarSigHelper() (no arguments). The no-args signature helper will require mono to implement ELEMENT_TYPE_INTERNAL in order to represent pointers to runtime types in ECMA IL signatures (which are just byte arrays) which is going to be a more involved project.

I have some notes in a Paper doc

@lambdageek lambdageek self-assigned this Feb 12, 2022
@marek-safar marek-safar merged commit a73eb7b into dotnet:main Mar 16, 2022
@marek-safar
Copy link
Contributor

Thank you!

@teo-tsirpanis teo-tsirpanis deleted the mono-ilgenerator branch March 16, 2022 10:06
radekdoulik pushed a commit to radekdoulik/runtime that referenced this pull request Mar 30, 2022
@ghost ghost locked as resolved and limited conversation to collaborators Apr 15, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area-VM-reflection-mono Reflection issues specific to MonoVM community-contribution Indicates that the PR has been added by a community member

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants