Fix several devirtualization issues #10371
Conversation
When doing interface devirtualization, if the object type is canonical, ensure that the owner type is too. The jit may present a mixed set when inlining a shared method into a non-shared method. Ideally the jit would also be able to present exact object types in such cases but currently it cannot guarantee this. Closes #10311. Adjust contracts to address some contract violations seen in desktop testing. Make the helper non-static and fold in some of the info that was passed from the caller to bring the desktop and CoreCLR implementations closer. Disallow interface devirt if the method is final but the class is not exact or final, since derived classes can still override final methods when implementing interfaces. Don't try and devirtualize interface calls from com objects. Add some related test cases.
|
@jkotas PTAL. This is preliminary but seems to fix issues where the jit doesn't have exact types. Not real happy with having to drop the final method check from interface call devirt, but right now I can't tell whether a method is an explicit interface implementation or an inherited one that happens to be marked sealed (the latter is probably rare but possible), and so relying just on the final bit on the method can lead to the wrong devirtualization. |
|
LGTM |
|
|
||
| // If the derived class is a shared class, make sure the | ||
| // owner class is too. | ||
| if (pDerivedMT->IsSharedByGenericInstantiations()) |
There was a problem hiding this comment.
Took me some time to convince myself that this should be ok ...
There was a problem hiding this comment.
Agree this is confusing. The jit isn't given or getting exact types in many cases. But when inlining a shared method into an unshared one, it ends up with the exact method type. So it can end up with an mixed view of types when devirtualizing a call in an inlinee.
For virtual calls this mixed view of types didn't matter since an exact type and its canonical type have the same vtable.
For invariant interface calls the mixed view likewise should not have mattered since the instance types do not figure in target resolution. I suspect one could add an equivalence check somewhere in the interface call resolution path that could fix the mixed case. However I was reluctant to make any changes there. Hence this fix.
By using shared types, we may lose the ability to devirtualize variant interface call since we can no longer reason about the relationships of the instance types. However it turns out we'd already lost those with the CanCastToInterface call above.
So the only diffs from this change are from no longer devirtualizing interface calls where the target is a final method; the changes on the VM side don't alter the jit's codegen.
Longer term I hope to selectively uplevel the types visible to the jit.
When doing interface devirtualization, if the object type is canonical,
ensure that the owner type is too. The jit may present a mixed set
when inlining a shared method into a non-shared method. Ideally the
jit would also be able to present exact object types in such cases but
currently it cannot guarantee this. Closes #10311.
Adjust contracts to address some contract violations seen in desktop
testing. Make the helper non-static and fold in some of the info that
was passed from the caller to bring the desktop and CoreCLR implementations
closer.
Disallow interface devirt if the method is final but the class is not
exact or final, since derived classes can still override final methods
when implementing interfaces.
Don't try and devirtualize interface calls from com objects.
Add some related test cases.