Skip to content

Conversation

@jkotas
Copy link
Member

@jkotas jkotas commented Jun 26, 2020

  • Delete copy&paste code from CoreCLR that is only relevant for IL verification
  • Delete early out for CORINFO_ACCESS_ADDRESS. There is no check like that in old crossgen. Reduces number of methods that fail to compile in CoreLib significantly.
  • Ship internal calls without throwing and catching exceptions.

Fixes #32663

- Delete copy&paste code from CoreCLR that is only relevant for IL verification
- Delete early out for CORINFO_ACCESS_ADDRESS. There is no check like that in old crossgen. Reduces number of methods that fail to compile in CoreLib significantly.
- Ship internal calls without throwing and catching exceptions.

Fixes dotnet#32663
@jkotas
Copy link
Member Author

jkotas commented Jun 26, 2020

cc @dotnet/crossgen-contrib

Factored out from #38229

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, thanks for cleaning this up!

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.

Please hold off on this for a day or two. I want to write up a test around the address stuff to make sure that it isn't broken in some odd way. There is code in zapinfo.cpp which checks for CORINFO_FIELD_STATIC_ADDRESS and skips codegen (see ZapInfo::getFieldInfo) I agree though that the current logic is way too broad.

@davidwrighton
Copy link
Member

I looked a little deeper, and you are right. That logic doesn't apply. We probably needed to disable the address handling due to incorrect layout, not any other reason. This is another reason to write that stress mode that I've been harping about where we can verify that layout in the runtime matches that of crossgen2. In fact, I'm going to write that instead of tests for these particular issues.

@jkotas jkotas merged commit 4e5c8c0 into dotnet:master Jun 27, 2020
@jkotas jkotas deleted the crossgen2-valuetype branch June 27, 2020 13:38
@ghost ghost locked as resolved and limited conversation to collaborators Dec 8, 2020
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.

Crossgen2 is missing support for CORINFO_FIELD_STATIC_ADDRESS

4 participants