This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] Devirtualization dump functions fix


On Thu, Jun 26, 2014 at 04:10:03PM +0200, Richard Biener wrote:
> 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.

But it increases overhead, there are hundreds of gimple_location calls
and most of them will never pass NULL.  Can't you simply
do what you do in the inline here in the couple of spots where
the stmt might be NULL?

	Jakub


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]