Fix PR58477, part II

Jan Hubicka hubicka@ucw.cz
Mon Dec 16 13:58:00 GMT 2013


> Hi,
> 
> On Sat, Dec 14, 2013 at 11:01:53PM +0100, Jan Hubicka wrote:
> > Hi,
> > this patch makes stmt_may_be_vtbl_ptr_store to skip clobbers as discussed
> > previously.
> 
> This is the first time I hear about this but the change is obviously
> OK, thanks.

It actually seems to solve quite number of false positives, not exactly sure why.
> 
> > Martin, I do not fully follow the logic of this function.  Can't we just
> > ignore all stores that are not of pointer type that looks like vptr
> > type?  All those tores are compiler generated and we probably can just
> > annotate them, right?
> 
> Richard wanted me to be quite conservative here and to a big extent I
> can see his point.  The AGGREGATE_TYPE_P is there because copies of
> the whole object could change the vptr in an unpredictable ways.  I
> doubt that this is what copy-constructors get expanded to but gimple
> does not provide any necessary guarantees.  Perhaps we could come up
> with an elaborate explanation why it cannot happen in valid C++ just
> as I did for the call statement (and that is written in the comment
> above the function) and then we could remove it.

OK, so the explanation is not as simple as claim that non-POD types
needs to be constructed or copied by constructor and C++ FE always
generate an explicit vtbl store?
> 
> Of course, having gimple defined and required annotation for vptr
> changes would be much better but then of course all transformations
> would have to make sure they preserve it.
> 
> IIRC, flag_strict_aliasing test was explicitely Richard's request,
> perhaps we could restrict the pointer type test further.
> 
> Does that answer your question?

Sort of, yes.  We should make some analysis how effective current
methods are (i.e. disabling it and checking how much devirtualization
improve for firefox) and if we find they seem insufficient, we probably
should think of better analysis or annotation...

Honza



More information about the Gcc-patches mailing list