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

Fix type generator test failures #44041

Merged
merged 11 commits into from
Nov 1, 2020

Conversation

davidwrighton
Copy link
Member

@davidwrighton davidwrighton commented Oct 30, 2020

  • Fix issue where method token referred to derived type but method was actually defined on base type

    • Previously the methods did not carry enough state to determine when to emit the owner type of the method, and which exact type was the owning type. The new logic computes it from the token in the presence of a proper instantiation context, which allows for correct operation.
  • Fix issue where constrained dispatch on a method of a valuetype would not put in the correct owner type

    • Issue fixed by determining if the token resolves to a method on the same type as the eventual target method, or if it needs to have a specific owning type specified

In general these issues where both caused by confusion around exactly the correct owning type, and it turned out that computation could not be computed within the signature emitter code, but instead needed to be computed in the JIT at point of use. Fortunately, we had a structure MethodWithToken that is used in these (and only these situations). Finally, I also made a pass through the emitter and related logic to remove various band-aids that had built up over the last few years to make all the tests and applications pass. I believe that the new logic should be correct in the general case.

Bonus tweak... Use parallelism when compiling the framework composite images with crossgen2, and fix bug in composite image generation where mangled symbol names might conflict.

Fixes #43466 and fixes #43467

…ived type but is actually defined on a base type

- Make MethodWithToken which is reliably IL token dependent always be generated in the CorInfoImpl class, and pass the appropriate context to the MethodWithToken constructor so that it can find the appropriate OwningType.
@Dotnet-GitSync-Bot Dotnet-GitSync-Bot added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Oct 30, 2020
@davidwrighton davidwrighton added area-crossgen2-coreclr and removed area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI labels Oct 30, 2020
@davidwrighton davidwrighton changed the title Fix crossgen2 issues where method reffered to derived type but method was actually defined on base type Fix crossgen2 issues where method referred to derived type but method was actually defined on base type Oct 30, 2020
@davidwrighton davidwrighton changed the title Fix crossgen2 issues where method referred to derived type but method was actually defined on base type Fix type generator test failures Oct 30, 2020
@davidwrighton davidwrighton requested a review from trylek October 30, 2020 23:40
@davidwrighton davidwrighton marked this pull request as ready for review October 30, 2020 23:41
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.

Looks great to me, I'm amazed by the cleanness and simplicity of your fixes, thank you!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
3 participants