Use results of whole program analysis to drive devirtualization - #5050
Conversation
|
Could it also switch interface params to concrete types where there is a single caller type? Though may not be worth it. |
Yes, I have that in my backlog already. Might be particularly useful for things like dependency injection where interfaces are basically only needed because of testability. It will need a RyuJIT change though. |
davidwrighton
left a comment
davidwrighton
left a comment
There was a problem hiding this comment.
I'm not entirely convinced the jit interface change is right, but the rest looks good. Since we don't have the same implementation concerns on CoreRT I'm ok with checking in what we have here, but be aware its not completely general purpose.
|
|
||
| // method or class might have the final bit | ||
| if (method.IsFinal || (owningMetadataType != null && owningMetadataType.IsSealed)) | ||
| if (_compilation.IsEffectivelySealed(method)) |
There was a problem hiding this comment.
I'm not convinced this is right. This will effectively result in methods being reported as sealed in the scanner phase, but not in the compiler phase. At least in the nutc world that could lead to failures.
There was a problem hiding this comment.
I think this is mitigated by the fact that both behaviors (whether to use the sealing information to devirtualize in the scanner already, and whether to report something that was sealed in the scanner as unsealed to the compiler) is fully wrapped within the scanner. The scanner as we have it here doesn't use the sealing information for anything.
If we were to reuse the scanner for different codegens with different scanning policies, this would be a place we make abstract/virtual. The scanner is not trying to be general purpose for all codegens; we'll have places we need to tweak.
3 participants
Split into 3 commits for better reviewability.
Fixes #5019. Besides fixing that, I also made this do "sealing" of types that we see never have a derived type allocated (because it was easy). This results in more than double the number of potential successful devirtualizations (for the web app template, we see an increase from 800 to 1800 cases where
resolveVirtualMethodreturns a method that is reported asfinal).