-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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
JIT: Fold more always failing type checks #97005
Conversation
Folding `x isinst B` where `x` is statically of type `A` is currently done via a call to `compareTypesForCast`. If it returns `TypeCompareState::Must` then we know that `A` can be cast to `B`, which also means any class derived from `A` can be cast to `B`, so we can always fold that. However, if it returns `TypeCompareState::MustNot` we only learn something if we know `x` is exactly `A`; otherwise, if `x` is more derived than `A`, it could still potentially be cast to `B`. In cases where the type hierarchies are distinct, however, we can still know that some type checks will fail. To detect the case add a query in the opposite direction; that is, if `B` cannot be cast to `A`, then the type hierarchies are distinct, so in that case we can fold it, regardless of having exact knowledge. This also requires some explicit checks for interface types, but perhaps we can move those to the EE side. Fix dotnet#97000
Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch Issue DetailsFolding In cases where the type hierarchies are distinct, however, we can still know that some type checks will fail. To detect the case add a query in the opposite direction; that is, if Fix #97000
|
// TODO: compareTypesForCast(interfaceType, otherType) returning | ||
// MustNot seems less useful than just having it return May. If we | ||
// change this on EE side we can avoid these getClassAttribs calls. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jkotas do you think it makes sense to change this in compareTypesForCast
? From what I understand, the current semantics of compareTypesForCast
is "if I have an object of exact type A, can it be cast to type B" -- which is a meaningless question when A
is an interface type. Alternatively, do you have any suggestions on better ways to check what we're after here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, looks like we fold Type.IsAssignableFrom
/Type.IsAssignableTo
using compareTypesForCast
as well, so seems like we cannot really change its behavior.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do you think it makes sense to change this in compareTypesForCast?
Yes, I think this logic should be better done on the EE side.
looks like we fold Type.IsAssignableFrom/Type.IsAssignableTo using compareTypesForCast as well, so seems like we cannot really change its behavior.
You can add a flag to the compareTypesForCast
or add a new JIT/EE interface method.
/azp run runtime-coreclr superpmi-diffs |
Azure Pipelines successfully started running 1 pipeline(s). |
Diffs after a collection on this branch. Pretty minor overall and mostly in tests. Doesn't seem worth the throughput cost or complexity of changing the JIT-EE interface. |
Folding
x isinst B
wherex
is statically of typeA
is currently done via a call tocompareTypesForCast
. If it returnsTypeCompareState::Must
then we know thatA
can be cast toB
, which also means any class derived fromA
can be cast toB
, so we can always fold that. However, if it returnsTypeCompareState::MustNot
we only learn something if we knowx
is exactlyA
; otherwise, ifx
is more derived thanA
, it could still potentially be cast toB
.In cases where the type hierarchies are disjoint, however, we can still know that some type checks will fail. To detect the case add a query in the opposite direction; that is, if
B
cannot be cast toA
, then the type hierarchies are disjoint, so in that case we can fold it, regardless of having exact knowledge. This also requires some explicit checks for interface types, but perhaps we can move those to the EE side.Fix #97000
Before:
After: