This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] Devirtualization dump functions fix
- From: Richard Biener <richard dot guenther at gmail dot com>
- To: Martin LiÅka <mliska at suse dot cz>
- Cc: GCC Patches <gcc-patches at gcc dot gnu dot org>, David Li <davidxl at google dot com>
- Date: Thu, 26 Jun 2014 16:10:03 +0200
- Subject: Re: [PATCH] Devirtualization dump functions fix
- Authentication-results: sourceware.org; auth=none
- References: <53AC19A7 dot 9020907 at suse dot cz> <CAFiYyc2Ocf=gUL-VDcvhba4T_fbmh7nqmcAm-PjvcL-90+np6w at mail dot gmail dot com> <53AC2386 dot 3030309 at suse dot cz>
On Thu, Jun 26, 2014 at 3:43 PM, Martin LiÅka <mliska@suse.cz> wrote:
>
> On 06/26/2014 03:20 PM, Richard Biener wrote:
>>
>> On Thu, Jun 26, 2014 at 3:01 PM, Martin LiÅka <mliska@suse.cz> wrote:
>>>
>>> Hello,
>>> I encountered similar issue to PR ipa/61462 where location_t locus =
>>> gimple_location (e->call_stmt) is called for e->call_stmt == NULL
>>> (Firefox
>>> with -flto -fdump-ipa-devirt). So that, I decided to introduce new
>>> function
>>> that is called for all potentially unsafe locations. I am wondering if a
>>> newly added function can be added in more seamless way (without playing
>>> with
>>> va_list and ATTRIBUTE_PRINTF stuff)?
>>>
>>> Bootstrapped and regtested on x86_64-unknown-linux-gnu.
>>
>> Hmm, I don't like that very much - dump_printf_loc_for_stmt still implies
>> stmt is not NULL. So you could have "fixed" gimple_location as well.
>> I suppose dump_printf_loc already does sth sane with UNKNOWN_LOCATION.
>>
>> Richard.
>
> Hi,
> you are right that it is quite complex change.
>
> Do you mean this one line change can be sufficient ?
> diff --git a/gcc/gimple.h b/gcc/gimple.h
> index ceefbc0..954195e 100644
> --- a/gcc/gimple.h
> +++ b/gcc/gimple.h
> @@ -1498,7 +1498,7 @@ gimple_set_block (gimple g, tree block)
> static inline location_t
> gimple_location (const_gimple g)
> {
> - return g->location;
> + return g ? g->location : UNKNOWN_LOCATION;
> }
>
> /* Return pointer to location information for statement G. */
>
> I will double-check if it solves the problem ;)
Well yes - it is of course similar broken in spirit but at least a lot
simpler ;) I'd put a comment there why we do check g for NULL.
Thanks,
Richard.
> Martin
>
>
>>
>>> Thanks,
>>> Martin
>>>
>>> ChangeLog:
>>>
>>> 2014-06-26 Martin Liska <mliska@suse.cz>
>>>
>>> * include/ansidecl.h: New collection of
>>> ATTRIBUTE_NULL_PRINTF_X_0
>>> defined.
>>>
>>> gcc/ChangeLog:
>>>
>>> 2014-06-26 Martin Liska <mliska@suse.cz>
>>>
>>> * dumpfile.h: New function dump_printf_loc_for_stmt.
>>> * dumpfile.c: Implementation added.
>>> (dump_vprintf): New function.i
>>> * cgraphunit.c: dump_printf_loc_for_stmt usage replaces
>>> dump_printf_loc.
>>> * gimple-fold.c: Likewise.
>>> * ipa-devirt.c: Likewise.
>>> * ipa-prop.c: Likewise.
>>> * ipa.c: Likewise.
>>> * tree-ssa-pre.c: Likewise.
>>>
>>>
>>>
>>>
>