Re: [GOOGLE] Disable -fdevirtualize by default

[Jan Hubicka <hubicka at ucw dot cz>]

> > In opts.c I have:
> >     case OPT_fprofile_use:
> >       if (!opts_set->x_flag_branch_probabilities)
> > ...
> >       /* Indirect call profiling should do all useful transformations
> >          speculative devirtualization does.  */
> >       if (!opts_set->x_flag_devirtualize_speculatively
> >           && opts->x_flag_value_profile_transformations)
> >         opts->x_flag_devirtualize_speculatively = false;
> >
> > so perhaps this hunk is somehow skipped with LIPO?
> 
> The 1.68% size reduction I measured is with plain O2 compilation, not LIPO.

OK, in that case I may look into trimming down the speculation with non-LTO compilation.
The code makes assumptions about completeness of type hiearchy that without LTO are
quite far reached. I tested it on firefox and there it seemed to do mostly
good job, so I decided to try to enable it for non-LTO, too.

It would be nice to understand what really happens in your case and if there
are lot of wrong speculations or if the speculated calls simply happens to not
be on hot paths in your benchmark.
> 
> >
> > Speculative devirtualization is somehwat less useful (may have more falce
> > positives) without LTO depending on how your headers are constructed.
> > It would be interesting to see if it does a lot of mistakes on your codebase.
> > (this can be easily done by forcing it to run with profile feedback, too and
> > it will tell you when its speculation differs from speculation already there).
> >>
> >> By the way, you mentioned 'hacking the
> >> ipa.c:walk_polymorphic_call_targets to not make the possible targets
> >> as
> >> reachable' -- is that something worth doing in trunk?   With that, we
> >> can probably just turn off speculative devirtualization.
> >
> > Well, the check is there to enable inlining. Disabling it for
> > -fprofile-generate will result in lost profile samples for virtual functions.
> > Disabling it by default will prevent inlining of devirtualized calls making
> > devirtualization not really useful.
> > Perhaps with LIPO situation is bit different because you bring in the other
> > module just to inline the call as you describe.
> 
> What I meant is whether it is suitable for plain build ? I have not
> looked at the details.

Yes, dropping the reachability walk should just work, only result in less
inlining.

Honza
> 
> >
> > One thing I can imagine doing is to make inliner consider the reachable
> > (in post-inlining sense, that is after removing extern inlines and virtual
> > functions) calls with priority and account only those to unit growth model.
> > This would make it more consistent over -fdevirtualize and more realistic
> > about resulting code size.
> >
> > I sort of considered this option but did not have any good data suggesting
> > I should implement it.
> >
> > In general it would be nice to understand this problem.  Also I plan to do
> > some retunning for 5.0 so it would be nice to know if you have other issues
> > with 4.9? (I did not closely followed Google branch changes, so if you can
> > point out those that are relevant for IPA tuning, I would be very interested
> > to see what problems you hit).
> 
> Ok I will collect a list of inlining related changes and let you know latter.
> 
> thanks,
> 
> David
> 
> >
> > Honza
> >>
> >> David
> >>
> >>
> >>
> >> >
> >> > Honza
> >> >>
> >> >> David
> >> >>
> >> >>
> >> >> >
> >> >> > Honza
> >> >> >>
> >> >> >> David
> >> >> >>
> >> >> >>
> >> >> >> On Sat, Oct 18, 2014 at 10:10 AM, Jan Hubicka <hubicka@ucw.cz> wrote:
> >> >> >> >> Disabling devirtualization reduces code size, both for instrumentation (because
> >> >> >> >> many more virtual functions are kept longer and therefore instrumented) and for
> >> >> >> >> normal optimization.
> >> >> >> >
> >> >> >> > OK, with profile instrumentation (that you seem to try to minimize) i can see
> >> >> >> > how you get noticeably more counters because virtual functions are kept longer.
> >> >> >> > (note that 4.9 is a lot more agressive on removing unreacable virtual functions
> >> >> >> > than earlier compilers).
> >> >> >> >
> >> >> >> > Instead of disabling -fdevirtualize completely (that will get you more indirect
> >> >> >> > calls and thus more topn profiling) you may consider just hacking
> >> >> >> > ipa.c:walk_polymorphic_call_targets to not make the possible targets as
> >> >> >> > reachable. (see the conditional on before_inlining_p).
> >> >> >> >
> >> >> >> > Of course this will get you less devirtualization (but with LTO the difference
> >> >> >> > should not be big - perhaps I could make switch for that for mainline) and less
> >> >> >> > accurate profiles when you get speculative devirtualization via topn.
> >> >> >> >
> >> >> >> > I would be very interested to see how much difference this makes.
> >> >> >> >
> >> >> >> > Honza