[swift-dev] [swift] master: Turn on dynamic super method dispatch by default (bd8d85d)

[David Farler]

> On Jan 18, 2016, at 22:19, Mark Lacey <mark.lacey at apple.com> wrote:
> 
> 
>> On Jan 18, 2016, at 9:56 PM, Chris Lattner via swift-dev <swift-dev at swift.org> wrote:
>> 
>> On Jan 18, 2016, at 6:36 PM, David Farler <dfarler at apple.com> wrote:
>>>>> 
>>>>> In SILGen, when the method isn't final, it does always emits a super_method instruction, but Roman added devirtualizer support for these, and it does look like they are getting optimized down to static dispatch when the nearest implementation is in the same module.
>>>> 
>>>> Good news, thanks.  Would it make sense for -O0 perf to have SILGen generate the static dispatch when possible, or is it too much trouble for the benefit?
>>>> 
>>>> -Chris
>>> 
>>> Actually it looks like it would be trivial to do it during SILGen, although looking at some generated code with TOT the devirtualizer is already kicking in for these at -Onone. Is there still a benefit from doing it earlier on?
>> 
>> I don’t know if it is worth it in this case.  The pro/con of doing this is usually:
>> 
>> 1) Faster generated -O0 builds makes everyone happy.
>> 2) Faster optimized compiles (by putting less pressure on the optimizer) makes people happy.
>> 3) Earlier devirtualization can have unexpected knock-on effects by allowing early inlining and better IPA to happen.
>> 4) more complexity in SILGen has a cost.  If it requires major new infrastructure, then it is usually not worth it.  If it drops in as a special case in an existing path, then it usually is.
>> 
>> I don’t know how the balance in this case works itself out, but both Clang IRGen and Swift SILGen (in other cases) try to avoid generating indirect calls when it is locally apparent they can be statically dispatched.
> 
> FWIW the fact that these are getting devirtualized right now is a happy accident of making @_transparent work in more cases by devirtualizing in the mandatory inliner. I don’t expect that to be removed, but if we really want to guarantee direct calls in the trivial cases it’s worth considering adding support explicitly in SILGen, especially if it’s trivial to do.
> 
> Mark

Interesting, I was just about to ask whether devirtualization was considered a mandatory optimization. So, it sounds like this is worth doing, it's a pretty lightweight check that I can drop into the two places where these are emitted. I think I have the patch - I'll run the tests and try to land it tomorrow. Thanks!

David