RE: [PATCH] Fix PR ipa/61190

[Bernd Edlinger <bernd dot edlinger at hotmail dot de>]

Hi,

On Mon, 2 Jun 2014 12:06:12, Richard Biener wrote:
>
> On Mon, Jun 2, 2014 at 11:00 AM, Bernd Edlinger
> <bernd.edlinger@hotmail.de> wrote:
>> Hi,
>>
>> the test case g++.old-deja/g++.mike/p4736b.C is mis-compiled with with all
>> optimization levels, except -O0 and -Og. This probably started with gcc 4.7.x.
>>
>> The constructor Main::Main() is first inlined and then completely optimized
>> away in the dce1 pass. But the thunk is still using the vtable, and therefore
>> crashes.
>>
>> Well, the problem seems to be, that the thunk code is
>> not emitted in the normal way using gimple code,
>> but instead by the i386 back-end with a callback.
>>
>> This makes the thunk invisible to the ipa-pure-const pass,
>> which assumes that all values just flow straight thu the thunk.
>>
>> See ipa-pure-const.c (analyze_function):
>>
>> if (fn->thunk.thunk_p || fn->alias)
>> {
>> /* Thunk gets propagated through, so nothing interesting happens. */
>> gcc_assert (ipa);
>> return l;
>> }
>>
>> But this is not true for a virtual base class:
>> The thunk itself references the vtable, so if nothing else might need
>> the vtable, the optimizer may remove the initalization of the vtable,
>> which happened in this example.
>>
>> The most simple work around would be the attached patch, which
>> simply falls back to emitting gimple code, if virtual base classes
>> are used.
>>
>> Boot-Strapped and Regression-Tested on x86_64-linux-gnu.
>> Ok for trunk?
>
> Honza should know better. I'd rather skip the above for
> thunks going the output_mi_thunk way.
>

Ahh Yes, that was of course my first attempt...

But there is not BB to enumerate in that case.
And the loop below just crashed in:

  FOR_EACH_BB_FN (this_block, cfun)
 

There is also another interesting thing to mention: when the
output_mi_thunk outputs the virtual thunk, there is no inlining at all.

But with this patch, the thunk inlines the called function,
and therefore the generated code looks rather nifty, compared to
the state before the patch.


Thanks
Bernd.

> That is, the target hook docs should be updated to reflect which
> kind of thunks it is supposed to handle.
>
> Richard.
>
>>
>> Thanks
>> Bernd.
>>