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.

Sign up for GitHub

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

Jump to bottom

Crossgen2 support for static virtual method resolution (take 2) #87438

Merged
merged 21 commits into from
Aug 8, 2023
Merged

Crossgen2 support for static virtual method resolution (take 2) #87438

merged 21 commits into from
Aug 8, 2023

Commits on Aug 8, 2023

  1. WIP: Crossgen2 support for static virtual method resolution 

    This is my initial attempt at porting the CoreCLR runtime code
for SVM resolution to Crossgen2. The TypeHierarchyTest now fails
at 66-th scenario. The biggest problems I'm hitting are around
making sure that we drop the constraint from the actual entrypoint
import cells, otherwise we end up with constructs like

BaseScenario61.Method() @ DerivedScenario61

and that crashes the runtime. I guess that my treatment of generics
is most likely still incorrect.

Thanks

Tomas

Address initial David's PR feedback

I'm adding this as a separate commit as it's basically just a
projection of David's PR suggestions without additional algorithmic
changes. I'll follow up with additional fixes for the remaining
test failures.

Thanks

Tomas

Address remaining David's PR feedback

Use existing virtual method algorithms per Michal's PR feedback

I have removed the previously added special virtual method algorithm
for static virtual method resolution and instead I added logic for
SVM resolution to the existing methods

ResolveInterfaceMethodToVirtualMethodOnType

and

ResolveVariantInterfaceMethodToVirtualMethodOnType

I haven't made any functional changes in this commit.

Thanks

Tomas

Remove the superfluous interfaceType parameter from DevirtualizationManager

Address additional Michal's and David's feedback; all tests pass

Several JIT interface fixes for static virtual methods

Don't drop constrained type for SVMs unresolved at compile time

With this change, more tests seem to be passing but there seems to
be a runtime problem ending up as an assertion: for instance,
in TypeHierarchyTests, Scenario 25, TryResolveConstraintMethodApprox
resolves the constrained call to

BaseScenario25<Func<String>>.Method()

which subsequently gets wrapped into an instantiating stub and
later crashes GC ref map check which seems to compare the GC ref map
of the wrapped method with the original import cell for the
constrained method. These naturally don't match as the wrapped
canonical method requires the method dictionary argument. I continue
investigating this but any insight into what exactly needs fixing here
would be more than welcome.

Thanks

Tomas

Use instantiating stubs for unresolved SVM import cells

Simplify the change by applying David's suggestion to not JIT unresolved SVM callers

Remove no longer used method AllowCompileTimeStaticVirtualMethodResolution
    @trylek
    trylek committed Aug 8, 2023
    Configuration menu
    Copy the full SHA
    0e68a2f View commit details
    Browse the repository at this point in the history

  2. Rebase against latest main; address parts of Michal's PR feedback

    @trylek
    trylek committed Aug 8, 2023
    Configuration menu
    Copy the full SHA
    8c69fcf View commit details
    Browse the repository at this point in the history

  3. Address additional Michal's PR feedback

    @trylek
    trylek committed Aug 8, 2023
    Configuration menu
    Copy the full SHA
    8513e6b View commit details
    Browse the repository at this point in the history

  4. Revert changes to MetadataVirtualMethodAlgorithm per Michal's PR feed… 

    …back
    @trylek
    trylek committed Aug 8, 2023
    Configuration menu
    Copy the full SHA
    475c190 View commit details
    Browse the repository at this point in the history

  5. Fix module token resolution in the presence of SVM devirtualization

    @trylek
    trylek committed Aug 8, 2023
    Configuration menu
    Copy the full SHA
    9fee598 View commit details
    Browse the repository at this point in the history

  6. Fix a few bugs found in TypeHierarchy tests

    @trylek
    trylek committed Aug 8, 2023
    Configuration menu
    Copy the full SHA
    8940fd1 View commit details
    Browse the repository at this point in the history

  7. Fix two Crossgen2 codegen bugs found via SVM testing 

    1. Delegate cctor helper JIT interface method was missing support
for type constraint.

2. Methods wrapped in instantiation / unboxing stubs shouldn't
claim they have a generic dictionary slot in their GC refmap.

Thanks

Tomas
    @trylek
    trylek committed Aug 8, 2023
    Configuration menu
    Copy the full SHA
    af42831 View commit details
    Browse the repository at this point in the history

  8. Fix bugs, address PR feedback

    @trylek
    trylek committed Aug 8, 2023
    Configuration menu
    Copy the full SHA
    a6785a4 View commit details
    Browse the repository at this point in the history

  9. Remove instrumentation; fix instantiation stub flag in getCallInfo

    @trylek
    trylek committed Aug 8, 2023
    Configuration menu
    Copy the full SHA
    bc60009 View commit details
    Browse the repository at this point in the history

  10. Revert change to Argiterator that turned out to be incorrect

    @trylek
    trylek committed Aug 8, 2023
    Configuration menu
    Copy the full SHA
    c75a1b0 View commit details
    Browse the repository at this point in the history

  11. Subtle fixes for SVM handling in ceeInfoGetCallInfo

    @trylek
    trylek committed Aug 8, 2023
    Configuration menu
    Copy the full SHA
    c1d0603 View commit details
    Browse the repository at this point in the history

  12. Fix delegate ctor and module token construction for SVMs

    @trylek
    trylek committed Aug 8, 2023
    Configuration menu
    Copy the full SHA
    0916bfc View commit details
    Browse the repository at this point in the history

  13. Remove instrumentation and more fixes for special SVM cases

    @trylek
    trylek committed Aug 8, 2023
    Configuration menu
    Copy the full SHA
    168908f View commit details
    Browse the repository at this point in the history

  14. Fix assertion failure seen in readytorun/tests/generic tests

    @trylek
    trylek committed Aug 8, 2023
    Configuration menu
    Copy the full SHA
    ad44935 View commit details
    Browse the repository at this point in the history

  15. Rebase against main; remove unused resource

    @trylek
    trylek committed Aug 8, 2023
    Configuration menu
    Copy the full SHA
    5bad02e View commit details
    Browse the repository at this point in the history

  16. Address Michal's PR feedback towards MethodImpl body lookup

    @trylek
    trylek committed Aug 8, 2023
    Configuration menu
    Copy the full SHA
    5ddc43c View commit details
    Browse the repository at this point in the history

  17. Fix typo and merge error

    @trylek
    trylek committed Aug 8, 2023
    Configuration menu
    Copy the full SHA
    f5f530c View commit details
    Browse the repository at this point in the history

  18. Add versioning resiliency test as suggested by DavidWr

    @trylek
    trylek committed Aug 8, 2023
    Configuration menu
    Copy the full SHA
    56754ea View commit details
    Browse the repository at this point in the history

  19. Remove superfluous blank lines

    @trylek
    trylek committed Aug 8, 2023
    Configuration menu
    Copy the full SHA
    8d642c6 View commit details
    Browse the repository at this point in the history

  20. Fix composite issues by introducing a new token kind

    @trylek
    trylek committed Aug 8, 2023
    Configuration menu
    Copy the full SHA
    26dd5cf View commit details
    Browse the repository at this point in the history

  21. Address David Wrighton's PR feedback 

    * Added stricter check to delegate constructor
* Disabled MethodBodyOnUnrelatedType test in Crossgen2 mode
* Reverted change to virtual method resolver

Thanks

Tomas
    @trylek
    trylek committed Aug 8, 2023
    Configuration menu
    Copy the full SHA
    1374c32 View commit details
    Browse the repository at this point in the history