Skip to content

Fix logic in compareTypesForCast #31681

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

Merged
merged 2 commits into from
Feb 4, 2020

Conversation

AndyAyersMS
Copy link
Member

We weren't careful enough with __Canon in some cases, which lead to unsafely
returning MustNot when the cast outcome was not determined at jit time.

Add an extra check, update comments, and add some test cases.

Addresses the failures seen in #1195 (which was reverted).

We weren't careful enough with `__Canon` in some cases, which lead to unsafely
returning `MustNot` when the cast outcome was not determined at jit time.

Add an extra check, update comments, and add some test cases.

Addresses the failures seen in dotnet#1195 (which was reverted).
@AndyAyersMS
Copy link
Member Author

cc @jkotas @EgorBo

No diffs seen in (x64 windows pmi Fx); current callers of checkTypesForCast not impacted.

@EgorBo
Copy link
Member

EgorBo commented Feb 3, 2020

NOTE: compareTypesForCast also always returns MayNot if toHnd is Nullable<> here https://github.com/dotnet/runtime/blob/master/src/coreclr/src/vm/jitinterface.cpp#L4474-L4477

probably makes sense to return MustNot if fromHnd is not a value type.

Copy link
Member

@jkotas jkotas left a comment

Choose a reason for hiding this comment

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

Thanks!

@AndyAyersMS
Copy link
Member Author

Formatting failure seems bogus; I did not change the code in the jit, and the patch file produced is empty.

@jkotas jkotas added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Feb 3, 2020
@AndyAyersMS
Copy link
Member Author

x86 libraries "System.Threading.Tasks.Tests Work Item" has failed twice now, leaving no diagnostic output to work with. Fairly sure this is unrelated to this change and it fails intermittently in master too.

Looks like this could be #2271 -- both failures in the above CI runs happen just after the 15 min mark.

image

@jkotas jkotas merged commit 41ec86b into dotnet:master Feb 4, 2020
@AndyAyersMS AndyAyersMS deleted the FixCompareTypesForCast branch February 4, 2020 07:44
@ghost ghost locked as resolved and limited conversation to collaborators Dec 10, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants