Re: [PATCH] Fix PR ipa/61190, updated

2014-11-25 Thread Jan Hubicka
> > Index: gcc/ipa-pure-const.c
> > ===
> > --- gcc/ipa-pure-const.c (revision 215888)
> > +++ gcc/ipa-pure-const.c (working copy)
> > @@ -744,6 +744,8 @@ analyze_function (struct cgraph_node *fn, bool ipa
> > {
> > /* Thunk gets propagated through, so nothing interesting happens. */
> > gcc_assert (ipa);
> > + if (fn->thunk.virtual_offset_p)
> > + l->pure_const_state = IPA_NEITHER;
> > return l;
> > }
> >
> 
> Hmm, I looked again at the above if statement, and I think now it should
> better be "if (fn->thunk.thunk_p && fn->thunk.virtual_offset_p)", because
> thunk.virtual_offset_p is probably not well defined if we come here because
> of fn->alias == true.

Yes, that is right.  I plan to put the other thunk info off the structure 
anyway.
> 
> > This makes the lattice to be initialized correctly, but you also need the
> > function_symbol calls that will skip thunks replaced by
> > something like function_or_non_virtual_thunk_symbol.
> >
> 
> Oh, I see what you mean, thanks.
> 
> I created a new method function_or_virtual_thunk_symbol() for this.
> And simplified the algorithm of both function_symbol variants a bit.
> 
> Attached, you'll find my updated patch for review.
> 
> Boot-strapped and regression tested on x86_64-linux-gnu.
> OK for trunk?
> 
> 
> Thanks
> Bernd.
> 
> > Can you, please, send the updated patch?
> > Sorry for late review,
> > Honza
> >
> 
> 

> 2014-11-25  Bernd Edlinger  
> 
>   PR ipa/61190
>   * cgraph.h (symtab_node::call_for_symbol_and_aliases): Fix comment.
>   (cgraph_node::function_or_virtual_thunk_symbol): New function.
>   (cgraph_node::call_for_symbol_and_aliases): Fix comment.
>   (cgraph_node::call_for_symbol_thunks_and_aliases): Adjust comment.
>   Add new optional parameter exclude_virtual_thunks.
>   * cgraph.c (cgraph_node::call_for_symbol_thunks_and_aliases): Add new
>   optional parameter exclude_virtual_thunks.
>   (cgraph_node::set_const_flag): Don't propagate to virtual thunks.
>   (cgraph_node::set_pure_flag): Likewise.
>   (cgraph_node::function_symbol): Simplified.
>   (cgraph_node::function_or_virtual_thunk_symbol): New function.
>   * ipa-pure-const.c (analyze_function): For virtual thunks set
>   pure_const_state to IPA_NEITHER.
>   (propagate_pure_const): Use function_or_virtual_thunk_symbol.

OK,
Honza
> 
> testsuite/ChangeLog:
> 2014-11-25  Bernd Edlinger  
> 
>   PR ipa/61190
>   * g++.old-deja/g++.mike/p4736b.C: Use -O2.
> 




RE: [PATCH] Fix PR ipa/61190, updated

2014-11-25 Thread Bernd Edlinger

Hi Honza,

On Mon, 24 Nov 2014 16:57:42 +0100, Jan Hubicka wrote:
>
> +cgraph_node::call_for_symbol_thunks_and_aliases_1 (bool (*callback)
> + (cgraph_node *, void *),
> + void *data,
> + bool include_overwritable,
> + bool exclude_virtual_thunks)
>
> Instead of adding _1 variant into public API, please just add implicit 
> agrumnet
> bool exclude_virtual_thunks=false into
> +cgraph_node::call_for_symbol_thunks_and_aliases

Ok, done.

> Index: gcc/ipa-pure-const.c
> ===
> --- gcc/ipa-pure-const.c (revision 215888)
> +++ gcc/ipa-pure-const.c (working copy)
> @@ -744,6 +744,8 @@ analyze_function (struct cgraph_node *fn, bool ipa
> {
> /* Thunk gets propagated through, so nothing interesting happens. */
> gcc_assert (ipa);
> + if (fn->thunk.virtual_offset_p)
> + l->pure_const_state = IPA_NEITHER;
> return l;
> }
>

Hmm, I looked again at the above if statement, and I think now it should
better be "if (fn->thunk.thunk_p && fn->thunk.virtual_offset_p)", because
thunk.virtual_offset_p is probably not well defined if we come here because
of fn->alias == true.

> This makes the lattice to be initialized correctly, but you also need the
> function_symbol calls that will skip thunks replaced by
> something like function_or_non_virtual_thunk_symbol.
>

Oh, I see what you mean, thanks.

I created a new method function_or_virtual_thunk_symbol() for this.
And simplified the algorithm of both function_symbol variants a bit.

Attached, you'll find my updated patch for review.

Boot-strapped and regression tested on x86_64-linux-gnu.
OK for trunk?


Thanks
Bernd.

> Can you, please, send the updated patch?
> Sorry for late review,
> Honza
>

  2014-11-25  Bernd Edlinger  

PR ipa/61190
* cgraph.h (symtab_node::call_for_symbol_and_aliases): Fix comment.
(cgraph_node::function_or_virtual_thunk_symbol): New function.
(cgraph_node::call_for_symbol_and_aliases): Fix comment.
(cgraph_node::call_for_symbol_thunks_and_aliases): Adjust comment.
Add new optional parameter exclude_virtual_thunks.
* cgraph.c (cgraph_node::call_for_symbol_thunks_and_aliases): Add new
optional parameter exclude_virtual_thunks.
(cgraph_node::set_const_flag): Don't propagate to virtual thunks.
(cgraph_node::set_pure_flag): Likewise.
(cgraph_node::function_symbol): Simplified.
(cgraph_node::function_or_virtual_thunk_symbol): New function.
* ipa-pure-const.c (analyze_function): For virtual thunks set
pure_const_state to IPA_NEITHER.
(propagate_pure_const): Use function_or_virtual_thunk_symbol.

testsuite/ChangeLog:
2014-11-25  Bernd Edlinger  

PR ipa/61190
* g++.old-deja/g++.mike/p4736b.C: Use -O2.



patch-pr61190.diff
Description: Binary data