Re: Fix PR58477, part II

[Richard Biener <richard dot guenther at gmail dot com>]

On Mon, Dec 16, 2013 at 2:58 PM, Jan Hubicka <hubicka@ucw.cz> wrote:
>> 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?

No as optimizers may combine stores for example.

>> 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