Re: PING^2: [PATCH] Don't mark IFUNC resolver as only called directly

2018-05-26 Thread H.J. Lu
On Thu, May 24, 2018 at 1:47 PM, H.J. Lu <hjl.to...@gmail.com> wrote:
> On Wed, May 23, 2018 at 8:35 AM, H.J. Lu <hjl.to...@gmail.com> wrote:
>> On Wed, May 23, 2018 at 8:11 AM, Jan Hubicka <hubi...@ucw.cz> wrote:
>>>> On Wed, May 23, 2018 at 2:01 AM, Jan Hubicka <hubi...@ucw.cz> wrote:
>>>> >> On Tue, May 22, 2018 at 9:21 AM, Jan Hubicka <hubi...@ucw.cz> wrote:
>>>> >> >> >>>>> >  class ipa_opt_pass_d;
>>>> >> >> >>>>> >  typedef ipa_opt_pass_d *ipa_opt_pass;
>>>> >> >> >>>>> > @@ -2894,7 +2896,8 @@ 
>>>> >> >> >>>>> > cgraph_node::only_called_directly_or_aliased_p (void)
>>>> >> >> >>>>> >   && !DECL_STATIC_CONSTRUCTOR (decl)
>>>> >> >> >>>>> >   && !DECL_STATIC_DESTRUCTOR (decl)
>>>> >> >> >>>>> >   && !used_from_object_file_p ()
>>>> >> >> >>>>> > - && !externally_visible);
>>>> >> >> >>>>> > + && !externally_visible
>>>> >> >> >>>>> > + && !lookup_attribute ("ifunc", DECL_ATTRIBUTES 
>>>> >> >> >>>>> > (decl)));
>>>> >> >> >>>>>
>>>> >> >> >>>>> How's it handled for our own generated resolver functions?  
>>>> >> >> >>>>> That is,
>>>> >> >> >>>>> isn't there sth cheaper than doing a lookup_attribute here?  I 
>>>> >> >> >>>>> see
>>>> >> >> >>>>> that make_dispatcher_decl nor 
>>>> >> >> >>>>> ix86_get_function_versions_dispatcher
>>>> >> >> >>>>> adds the 'ifunc' attribute (though they are TREE_PUBLIC there).
>>>> >> >> >>>>
>>>> >> >> >>>> Is there any drawback of setting force_output flag?
>>>> >> >> >>>> Honza
>>>> >> >> >>>
>>>> >> >> >>> Setting force_output may prevent some optimizations.  Can we add 
>>>> >> >> >>> a bit
>>>> >> >> >>> for IFUNC resolver?
>>>> >> >> >>>
>>>> >> >> >>
>>>> >> >> >> Here is the patch to add ifunc_resolver to cgraph_node. Tested on 
>>>> >> >> >> x86-64
>>>> >> >> >> and i686.  Any comments?
>>>> >> >> >>
>>>> >> >> >
>>>> >> >> > PING:
>>>> >> >> >
>>>> >> >> > https://gcc.gnu.org/ml/gcc-patches/2018-04/msg00647.html
>>>> >> >> >
>>>> >> >>
>>>> >> >> PING.
>>>> >> > OK, but please extend the verifier that ifunc_resolver flag is 
>>>> >> > equivalent to
>>>> >> > lookup_attribute ("ifunc", DECL_ATTRIBUTES (decl))
>>>> >> > so we are sure things stays in sync.
>>>> >> >
>>>> >>
>>>> >> Like this
>>>> >>
>>>> >> diff --git a/gcc/symtab.c b/gcc/symtab.c
>>>> >> index 80f6f910c3b..954920b6dff 100644
>>>> >> --- a/gcc/symtab.c
>>>> >> +++ b/gcc/symtab.c
>>>> >> @@ -998,6 +998,13 @@ symtab_node::verify_base (void)
>>>> >>error ("function symbol is not function");
>>>> >>error_found = true;
>>>> >>}
>>>> >> +  else if ((lookup_attribute ("ifunc", DECL_ATTRIBUTES (decl))
>>>> >> + != NULL)
>>>> >> + != dyn_cast  (this)->ifunc_resolver)
>>>> >> +  {
>>>> >> +  error ("inconsistent `ifunc' attribute");
>>>> >> +  error_found = true;
>>>> >> +  }
>>>> >>  }
>>>> >>else if (is_a  (this))
>>>> >>  {
>>>> >>
>>>> >>
>

Re: PING^2: [PATCH] Don't mark IFUNC resolver as only called directly

2018-05-24 Thread H.J. Lu
On Wed, May 23, 2018 at 8:35 AM, H.J. Lu <hjl.to...@gmail.com> wrote:
> On Wed, May 23, 2018 at 8:11 AM, Jan Hubicka <hubi...@ucw.cz> wrote:
>>> On Wed, May 23, 2018 at 2:01 AM, Jan Hubicka <hubi...@ucw.cz> wrote:
>>> >> On Tue, May 22, 2018 at 9:21 AM, Jan Hubicka <hubi...@ucw.cz> wrote:
>>> >> >> >>>>> >  class ipa_opt_pass_d;
>>> >> >> >>>>> >  typedef ipa_opt_pass_d *ipa_opt_pass;
>>> >> >> >>>>> > @@ -2894,7 +2896,8 @@ 
>>> >> >> >>>>> > cgraph_node::only_called_directly_or_aliased_p (void)
>>> >> >> >>>>> >   && !DECL_STATIC_CONSTRUCTOR (decl)
>>> >> >> >>>>> >   && !DECL_STATIC_DESTRUCTOR (decl)
>>> >> >> >>>>> >   && !used_from_object_file_p ()
>>> >> >> >>>>> > - && !externally_visible);
>>> >> >> >>>>> > + && !externally_visible
>>> >> >> >>>>> > + && !lookup_attribute ("ifunc", DECL_ATTRIBUTES 
>>> >> >> >>>>> > (decl)));
>>> >> >> >>>>>
>>> >> >> >>>>> How's it handled for our own generated resolver functions?  
>>> >> >> >>>>> That is,
>>> >> >> >>>>> isn't there sth cheaper than doing a lookup_attribute here?  I 
>>> >> >> >>>>> see
>>> >> >> >>>>> that make_dispatcher_decl nor 
>>> >> >> >>>>> ix86_get_function_versions_dispatcher
>>> >> >> >>>>> adds the 'ifunc' attribute (though they are TREE_PUBLIC there).
>>> >> >> >>>>
>>> >> >> >>>> Is there any drawback of setting force_output flag?
>>> >> >> >>>> Honza
>>> >> >> >>>
>>> >> >> >>> Setting force_output may prevent some optimizations.  Can we add 
>>> >> >> >>> a bit
>>> >> >> >>> for IFUNC resolver?
>>> >> >> >>>
>>> >> >> >>
>>> >> >> >> Here is the patch to add ifunc_resolver to cgraph_node. Tested on 
>>> >> >> >> x86-64
>>> >> >> >> and i686.  Any comments?
>>> >> >> >>
>>> >> >> >
>>> >> >> > PING:
>>> >> >> >
>>> >> >> > https://gcc.gnu.org/ml/gcc-patches/2018-04/msg00647.html
>>> >> >> >
>>> >> >>
>>> >> >> PING.
>>> >> > OK, but please extend the verifier that ifunc_resolver flag is 
>>> >> > equivalent to
>>> >> > lookup_attribute ("ifunc", DECL_ATTRIBUTES (decl))
>>> >> > so we are sure things stays in sync.
>>> >> >
>>> >>
>>> >> Like this
>>> >>
>>> >> diff --git a/gcc/symtab.c b/gcc/symtab.c
>>> >> index 80f6f910c3b..954920b6dff 100644
>>> >> --- a/gcc/symtab.c
>>> >> +++ b/gcc/symtab.c
>>> >> @@ -998,6 +998,13 @@ symtab_node::verify_base (void)
>>> >>error ("function symbol is not function");
>>> >>error_found = true;
>>> >>}
>>> >> +  else if ((lookup_attribute ("ifunc", DECL_ATTRIBUTES (decl))
>>> >> + != NULL)
>>> >> + != dyn_cast  (this)->ifunc_resolver)
>>> >> +  {
>>> >> +  error ("inconsistent `ifunc' attribute");
>>> >> +  error_found = true;
>>> >> +  }
>>> >>  }
>>> >>else if (is_a  (this))
>>> >>  {
>>> >>
>>> >>
>>> >> Thanks.
>>> > Yes, thanks!
>>> > Honza
>>>
>>> I'd like to also fix it on GCC 8 branch for CET.  Should I backport my
>>> patch to GCC 8 after a few days or use the simple patch for GCC 8:
>>>
>>> https://gcc.gnu.org/ml/gcc-patches/2018-04/msg00588.html
>>
>> I wou

Re: PING^2: [PATCH] Don't mark IFUNC resolver as only called directly

2018-05-23 Thread H.J. Lu
On Wed, May 23, 2018 at 8:11 AM, Jan Hubicka <hubi...@ucw.cz> wrote:
>> On Wed, May 23, 2018 at 2:01 AM, Jan Hubicka <hubi...@ucw.cz> wrote:
>> >> On Tue, May 22, 2018 at 9:21 AM, Jan Hubicka <hubi...@ucw.cz> wrote:
>> >> >> >>>>> >  class ipa_opt_pass_d;
>> >> >> >>>>> >  typedef ipa_opt_pass_d *ipa_opt_pass;
>> >> >> >>>>> > @@ -2894,7 +2896,8 @@ 
>> >> >> >>>>> > cgraph_node::only_called_directly_or_aliased_p (void)
>> >> >> >>>>> >   && !DECL_STATIC_CONSTRUCTOR (decl)
>> >> >> >>>>> >   && !DECL_STATIC_DESTRUCTOR (decl)
>> >> >> >>>>> >   && !used_from_object_file_p ()
>> >> >> >>>>> > - && !externally_visible);
>> >> >> >>>>> > + && !externally_visible
>> >> >> >>>>> > + && !lookup_attribute ("ifunc", DECL_ATTRIBUTES 
>> >> >> >>>>> > (decl)));
>> >> >> >>>>>
>> >> >> >>>>> How's it handled for our own generated resolver functions?  That 
>> >> >> >>>>> is,
>> >> >> >>>>> isn't there sth cheaper than doing a lookup_attribute here?  I 
>> >> >> >>>>> see
>> >> >> >>>>> that make_dispatcher_decl nor 
>> >> >> >>>>> ix86_get_function_versions_dispatcher
>> >> >> >>>>> adds the 'ifunc' attribute (though they are TREE_PUBLIC there).
>> >> >> >>>>
>> >> >> >>>> Is there any drawback of setting force_output flag?
>> >> >> >>>> Honza
>> >> >> >>>
>> >> >> >>> Setting force_output may prevent some optimizations.  Can we add a 
>> >> >> >>> bit
>> >> >> >>> for IFUNC resolver?
>> >> >> >>>
>> >> >> >>
>> >> >> >> Here is the patch to add ifunc_resolver to cgraph_node. Tested on 
>> >> >> >> x86-64
>> >> >> >> and i686.  Any comments?
>> >> >> >>
>> >> >> >
>> >> >> > PING:
>> >> >> >
>> >> >> > https://gcc.gnu.org/ml/gcc-patches/2018-04/msg00647.html
>> >> >> >
>> >> >>
>> >> >> PING.
>> >> > OK, but please extend the verifier that ifunc_resolver flag is 
>> >> > equivalent to
>> >> > lookup_attribute ("ifunc", DECL_ATTRIBUTES (decl))
>> >> > so we are sure things stays in sync.
>> >> >
>> >>
>> >> Like this
>> >>
>> >> diff --git a/gcc/symtab.c b/gcc/symtab.c
>> >> index 80f6f910c3b..954920b6dff 100644
>> >> --- a/gcc/symtab.c
>> >> +++ b/gcc/symtab.c
>> >> @@ -998,6 +998,13 @@ symtab_node::verify_base (void)
>> >>error ("function symbol is not function");
>> >>error_found = true;
>> >>}
>> >> +  else if ((lookup_attribute ("ifunc", DECL_ATTRIBUTES (decl))
>> >> + != NULL)
>> >> + != dyn_cast  (this)->ifunc_resolver)
>> >> +  {
>> >> +  error ("inconsistent `ifunc' attribute");
>> >> +  error_found = true;
>> >> +  }
>> >>  }
>> >>else if (is_a  (this))
>> >>  {
>> >>
>> >>
>> >> Thanks.
>> > Yes, thanks!
>> > Honza
>>
>> I'd like to also fix it on GCC 8 branch for CET.  Should I backport my
>> patch to GCC 8 after a few days or use the simple patch for GCC 8:
>>
>> https://gcc.gnu.org/ml/gcc-patches/2018-04/msg00588.html
>
> I would backport this one so we don't unnecesarily diverge.
> Thanks!
> Honza

This is the backport which I will check into GCC 8 branch next week.

Thanks.

-- 
H.J.
From a938f6318261eb93cb02ede65b0966c2d8a78880 Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.to...@gmail.com>
Date: Wed, 11 Apr 2018 12:31:21 -0700
Subject: [PATCH] Don't mark IFUNC resolver as only called directly

Si

Re: PING^2: [PATCH] Don't mark IFUNC resolver as only called directly

2018-05-23 Thread Jan Hubicka
> On Wed, May 23, 2018 at 2:01 AM, Jan Hubicka  wrote:
> >> On Tue, May 22, 2018 at 9:21 AM, Jan Hubicka  wrote:
> >> >> > >  class ipa_opt_pass_d;
> >> >> > >  typedef ipa_opt_pass_d *ipa_opt_pass;
> >> >> > > @@ -2894,7 +2896,8 @@ 
> >> >> > > cgraph_node::only_called_directly_or_aliased_p (void)
> >> >> > >   && !DECL_STATIC_CONSTRUCTOR (decl)
> >> >> > >   && !DECL_STATIC_DESTRUCTOR (decl)
> >> >> > >   && !used_from_object_file_p ()
> >> >> > > - && !externally_visible);
> >> >> > > + && !externally_visible
> >> >> > > + && !lookup_attribute ("ifunc", DECL_ATTRIBUTES 
> >> >> > > (decl)));
> >> >> >
> >> >> > How's it handled for our own generated resolver functions?  That 
> >> >> > is,
> >> >> > isn't there sth cheaper than doing a lookup_attribute here?  I see
> >> >> > that make_dispatcher_decl nor 
> >> >> > ix86_get_function_versions_dispatcher
> >> >> > adds the 'ifunc' attribute (though they are TREE_PUBLIC there).
> >> >> 
> >> >>  Is there any drawback of setting force_output flag?
> >> >>  Honza
> >> >> >>>
> >> >> >>> Setting force_output may prevent some optimizations.  Can we add a 
> >> >> >>> bit
> >> >> >>> for IFUNC resolver?
> >> >> >>>
> >> >> >>
> >> >> >> Here is the patch to add ifunc_resolver to cgraph_node. Tested on 
> >> >> >> x86-64
> >> >> >> and i686.  Any comments?
> >> >> >>
> >> >> >
> >> >> > PING:
> >> >> >
> >> >> > https://gcc.gnu.org/ml/gcc-patches/2018-04/msg00647.html
> >> >> >
> >> >>
> >> >> PING.
> >> > OK, but please extend the verifier that ifunc_resolver flag is 
> >> > equivalent to
> >> > lookup_attribute ("ifunc", DECL_ATTRIBUTES (decl))
> >> > so we are sure things stays in sync.
> >> >
> >>
> >> Like this
> >>
> >> diff --git a/gcc/symtab.c b/gcc/symtab.c
> >> index 80f6f910c3b..954920b6dff 100644
> >> --- a/gcc/symtab.c
> >> +++ b/gcc/symtab.c
> >> @@ -998,6 +998,13 @@ symtab_node::verify_base (void)
> >>error ("function symbol is not function");
> >>error_found = true;
> >>}
> >> +  else if ((lookup_attribute ("ifunc", DECL_ATTRIBUTES (decl))
> >> + != NULL)
> >> + != dyn_cast  (this)->ifunc_resolver)
> >> +  {
> >> +  error ("inconsistent `ifunc' attribute");
> >> +  error_found = true;
> >> +  }
> >>  }
> >>else if (is_a  (this))
> >>  {
> >>
> >>
> >> Thanks.
> > Yes, thanks!
> > Honza
> 
> I'd like to also fix it on GCC 8 branch for CET.  Should I backport my
> patch to GCC 8 after a few days or use the simple patch for GCC 8:
> 
> https://gcc.gnu.org/ml/gcc-patches/2018-04/msg00588.html

I would backport this one so we don't unnecesarily diverge.
Thanks!
Honza
> 
> Thanks.
> 
> -- 
> H.J.


Re: PING^2: [PATCH] Don't mark IFUNC resolver as only called directly

2018-05-23 Thread H.J. Lu
On Wed, May 23, 2018 at 2:01 AM, Jan Hubicka  wrote:
>> On Tue, May 22, 2018 at 9:21 AM, Jan Hubicka  wrote:
>> >> > >  class ipa_opt_pass_d;
>> >> > >  typedef ipa_opt_pass_d *ipa_opt_pass;
>> >> > > @@ -2894,7 +2896,8 @@ 
>> >> > > cgraph_node::only_called_directly_or_aliased_p (void)
>> >> > >   && !DECL_STATIC_CONSTRUCTOR (decl)
>> >> > >   && !DECL_STATIC_DESTRUCTOR (decl)
>> >> > >   && !used_from_object_file_p ()
>> >> > > - && !externally_visible);
>> >> > > + && !externally_visible
>> >> > > + && !lookup_attribute ("ifunc", DECL_ATTRIBUTES (decl)));
>> >> >
>> >> > How's it handled for our own generated resolver functions?  That is,
>> >> > isn't there sth cheaper than doing a lookup_attribute here?  I see
>> >> > that make_dispatcher_decl nor ix86_get_function_versions_dispatcher
>> >> > adds the 'ifunc' attribute (though they are TREE_PUBLIC there).
>> >> 
>> >>  Is there any drawback of setting force_output flag?
>> >>  Honza
>> >> >>>
>> >> >>> Setting force_output may prevent some optimizations.  Can we add a bit
>> >> >>> for IFUNC resolver?
>> >> >>>
>> >> >>
>> >> >> Here is the patch to add ifunc_resolver to cgraph_node. Tested on 
>> >> >> x86-64
>> >> >> and i686.  Any comments?
>> >> >>
>> >> >
>> >> > PING:
>> >> >
>> >> > https://gcc.gnu.org/ml/gcc-patches/2018-04/msg00647.html
>> >> >
>> >>
>> >> PING.
>> > OK, but please extend the verifier that ifunc_resolver flag is equivalent 
>> > to
>> > lookup_attribute ("ifunc", DECL_ATTRIBUTES (decl))
>> > so we are sure things stays in sync.
>> >
>>
>> Like this
>>
>> diff --git a/gcc/symtab.c b/gcc/symtab.c
>> index 80f6f910c3b..954920b6dff 100644
>> --- a/gcc/symtab.c
>> +++ b/gcc/symtab.c
>> @@ -998,6 +998,13 @@ symtab_node::verify_base (void)
>>error ("function symbol is not function");
>>error_found = true;
>>}
>> +  else if ((lookup_attribute ("ifunc", DECL_ATTRIBUTES (decl))
>> + != NULL)
>> + != dyn_cast  (this)->ifunc_resolver)
>> +  {
>> +  error ("inconsistent `ifunc' attribute");
>> +  error_found = true;
>> +  }
>>  }
>>else if (is_a  (this))
>>  {
>>
>>
>> Thanks.
> Yes, thanks!
> Honza

I'd like to also fix it on GCC 8 branch for CET.  Should I backport my
patch to GCC 8 after a few days or use the simple patch for GCC 8:

https://gcc.gnu.org/ml/gcc-patches/2018-04/msg00588.html

Thanks.

-- 
H.J.


Re: PING^2: [PATCH] Don't mark IFUNC resolver as only called directly

2018-05-23 Thread Jan Hubicka
> On Tue, May 22, 2018 at 9:21 AM, Jan Hubicka  wrote:
> >> > >  class ipa_opt_pass_d;
> >> > >  typedef ipa_opt_pass_d *ipa_opt_pass;
> >> > > @@ -2894,7 +2896,8 @@ 
> >> > > cgraph_node::only_called_directly_or_aliased_p (void)
> >> > >   && !DECL_STATIC_CONSTRUCTOR (decl)
> >> > >   && !DECL_STATIC_DESTRUCTOR (decl)
> >> > >   && !used_from_object_file_p ()
> >> > > - && !externally_visible);
> >> > > + && !externally_visible
> >> > > + && !lookup_attribute ("ifunc", DECL_ATTRIBUTES (decl)));
> >> >
> >> > How's it handled for our own generated resolver functions?  That is,
> >> > isn't there sth cheaper than doing a lookup_attribute here?  I see
> >> > that make_dispatcher_decl nor ix86_get_function_versions_dispatcher
> >> > adds the 'ifunc' attribute (though they are TREE_PUBLIC there).
> >> 
> >>  Is there any drawback of setting force_output flag?
> >>  Honza
> >> >>>
> >> >>> Setting force_output may prevent some optimizations.  Can we add a bit
> >> >>> for IFUNC resolver?
> >> >>>
> >> >>
> >> >> Here is the patch to add ifunc_resolver to cgraph_node. Tested on x86-64
> >> >> and i686.  Any comments?
> >> >>
> >> >
> >> > PING:
> >> >
> >> > https://gcc.gnu.org/ml/gcc-patches/2018-04/msg00647.html
> >> >
> >>
> >> PING.
> > OK, but please extend the verifier that ifunc_resolver flag is equivalent to
> > lookup_attribute ("ifunc", DECL_ATTRIBUTES (decl))
> > so we are sure things stays in sync.
> >
> 
> Like this
> 
> diff --git a/gcc/symtab.c b/gcc/symtab.c
> index 80f6f910c3b..954920b6dff 100644
> --- a/gcc/symtab.c
> +++ b/gcc/symtab.c
> @@ -998,6 +998,13 @@ symtab_node::verify_base (void)
>error ("function symbol is not function");
>error_found = true;
>}
> +  else if ((lookup_attribute ("ifunc", DECL_ATTRIBUTES (decl))
> + != NULL)
> + != dyn_cast  (this)->ifunc_resolver)
> +  {
> +  error ("inconsistent `ifunc' attribute");
> +  error_found = true;
> +  }
>  }
>else if (is_a  (this))
>  {
> 
> 
> Thanks.
Yes, thanks!
Honza
> 
> -- 
> H.J.


Re: PING^2: [PATCH] Don't mark IFUNC resolver as only called directly

2018-05-22 Thread H.J. Lu
On Tue, May 22, 2018 at 9:21 AM, Jan Hubicka  wrote:
>> > >  class ipa_opt_pass_d;
>> > >  typedef ipa_opt_pass_d *ipa_opt_pass;
>> > > @@ -2894,7 +2896,8 @@ cgraph_node::only_called_directly_or_aliased_p 
>> > > (void)
>> > >   && !DECL_STATIC_CONSTRUCTOR (decl)
>> > >   && !DECL_STATIC_DESTRUCTOR (decl)
>> > >   && !used_from_object_file_p ()
>> > > - && !externally_visible);
>> > > + && !externally_visible
>> > > + && !lookup_attribute ("ifunc", DECL_ATTRIBUTES (decl)));
>> >
>> > How's it handled for our own generated resolver functions?  That is,
>> > isn't there sth cheaper than doing a lookup_attribute here?  I see
>> > that make_dispatcher_decl nor ix86_get_function_versions_dispatcher
>> > adds the 'ifunc' attribute (though they are TREE_PUBLIC there).
>> 
>>  Is there any drawback of setting force_output flag?
>>  Honza
>> >>>
>> >>> Setting force_output may prevent some optimizations.  Can we add a bit
>> >>> for IFUNC resolver?
>> >>>
>> >>
>> >> Here is the patch to add ifunc_resolver to cgraph_node. Tested on x86-64
>> >> and i686.  Any comments?
>> >>
>> >
>> > PING:
>> >
>> > https://gcc.gnu.org/ml/gcc-patches/2018-04/msg00647.html
>> >
>>
>> PING.
> OK, but please extend the verifier that ifunc_resolver flag is equivalent to
> lookup_attribute ("ifunc", DECL_ATTRIBUTES (decl))
> so we are sure things stays in sync.
>

Like this

diff --git a/gcc/symtab.c b/gcc/symtab.c
index 80f6f910c3b..954920b6dff 100644
--- a/gcc/symtab.c
+++ b/gcc/symtab.c
@@ -998,6 +998,13 @@ symtab_node::verify_base (void)
   error ("function symbol is not function");
   error_found = true;
   }
+  else if ((lookup_attribute ("ifunc", DECL_ATTRIBUTES (decl))
+ != NULL)
+ != dyn_cast  (this)->ifunc_resolver)
+  {
+  error ("inconsistent `ifunc' attribute");
+  error_found = true;
+  }
 }
   else if (is_a  (this))
 {


Thanks.

-- 
H.J.


Re: PING^2: [PATCH] Don't mark IFUNC resolver as only called directly

2018-05-22 Thread Jan Hubicka
> > >  class ipa_opt_pass_d;
> > >  typedef ipa_opt_pass_d *ipa_opt_pass;
> > > @@ -2894,7 +2896,8 @@ cgraph_node::only_called_directly_or_aliased_p 
> > > (void)
> > >   && !DECL_STATIC_CONSTRUCTOR (decl)
> > >   && !DECL_STATIC_DESTRUCTOR (decl)
> > >   && !used_from_object_file_p ()
> > > - && !externally_visible);
> > > + && !externally_visible
> > > + && !lookup_attribute ("ifunc", DECL_ATTRIBUTES (decl)));
> >
> > How's it handled for our own generated resolver functions?  That is,
> > isn't there sth cheaper than doing a lookup_attribute here?  I see
> > that make_dispatcher_decl nor ix86_get_function_versions_dispatcher
> > adds the 'ifunc' attribute (though they are TREE_PUBLIC there).
> 
>  Is there any drawback of setting force_output flag?
>  Honza
> >>>
> >>> Setting force_output may prevent some optimizations.  Can we add a bit
> >>> for IFUNC resolver?
> >>>
> >>
> >> Here is the patch to add ifunc_resolver to cgraph_node. Tested on x86-64
> >> and i686.  Any comments?
> >>
> >
> > PING:
> >
> > https://gcc.gnu.org/ml/gcc-patches/2018-04/msg00647.html
> >
> 
> PING.
OK, but please extend the verifier that ifunc_resolver flag is equivalent to
lookup_attribute ("ifunc", DECL_ATTRIBUTES (decl))
so we are sure things stays in sync.

Thanks and sorry for the delay,
Honza
> 
> 
> -- 
> H.J.


PING^2: [PATCH] Don't mark IFUNC resolver as only called directly

2018-05-14 Thread H.J. Lu
On Wed, Apr 25, 2018 at 8:49 PM, H.J. Lu  wrote:
> On Thu, Apr 12, 2018 at 3:50 PM, H.J. Lu  wrote:
>> On Thu, Apr 12, 2018 at 6:39 AM, H.J. Lu  wrote:
>>> On Thu, Apr 12, 2018 at 5:17 AM, Jan Hubicka  wrote:
> On Thu, Apr 12, 2018 at 1:29 PM, H.J. Lu  wrote:
> > Since IFUNC resolver is called indirectly, don't mark IFUNC resolver as
> > only called directly.
> >
> > OK for trunk?
> >
> >
> > H.J.
> > ---
> > gcc/
> >
> > PR target/85345
> > * cgraph.h: Include stringpool.h" and "attribs.h".
> > (cgraph_node::only_called_directly_or_aliased_p): Return false
> > for IFUNC resolver.
> >
> > gcc/testsuite/
> >
> > PR target/85345
> > * gcc.target/i386/pr85345.c: New test.
> > ---
> >  gcc/cgraph.h|  5 +++-
> >  gcc/testsuite/gcc.target/i386/pr85345.c | 44 
> > +
> >  2 files changed, 48 insertions(+), 1 deletion(-)
> >  create mode 100644 gcc/testsuite/gcc.target/i386/pr85345.c
> >
> > diff --git a/gcc/cgraph.h b/gcc/cgraph.h
> > index d1ef8408497..9e195824fcc 100644
> > --- a/gcc/cgraph.h
> > +++ b/gcc/cgraph.h
> > @@ -24,6 +24,8 @@ along with GCC; see the file COPYING3.  If not see
> >  #include "profile-count.h"
> >  #include "ipa-ref.h"
> >  #include "plugin-api.h"
> > +#include "stringpool.h"
> > +#include "attribs.h"
> >
> >  class ipa_opt_pass_d;
> >  typedef ipa_opt_pass_d *ipa_opt_pass;
> > @@ -2894,7 +2896,8 @@ cgraph_node::only_called_directly_or_aliased_p 
> > (void)
> >   && !DECL_STATIC_CONSTRUCTOR (decl)
> >   && !DECL_STATIC_DESTRUCTOR (decl)
> >   && !used_from_object_file_p ()
> > - && !externally_visible);
> > + && !externally_visible
> > + && !lookup_attribute ("ifunc", DECL_ATTRIBUTES (decl)));
>
> How's it handled for our own generated resolver functions?  That is,
> isn't there sth cheaper than doing a lookup_attribute here?  I see
> that make_dispatcher_decl nor ix86_get_function_versions_dispatcher
> adds the 'ifunc' attribute (though they are TREE_PUBLIC there).

 Is there any drawback of setting force_output flag?
 Honza
>>>
>>> Setting force_output may prevent some optimizations.  Can we add a bit
>>> for IFUNC resolver?
>>>
>>
>> Here is the patch to add ifunc_resolver to cgraph_node. Tested on x86-64
>> and i686.  Any comments?
>>
>
> PING:
>
> https://gcc.gnu.org/ml/gcc-patches/2018-04/msg00647.html
>

PING.


-- 
H.J.


PING: [PATCH] Don't mark IFUNC resolver as only called directly

2018-04-25 Thread H.J. Lu
On Thu, Apr 12, 2018 at 3:50 PM, H.J. Lu  wrote:
> On Thu, Apr 12, 2018 at 6:39 AM, H.J. Lu  wrote:
>> On Thu, Apr 12, 2018 at 5:17 AM, Jan Hubicka  wrote:
 On Thu, Apr 12, 2018 at 1:29 PM, H.J. Lu  wrote:
 > Since IFUNC resolver is called indirectly, don't mark IFUNC resolver as
 > only called directly.
 >
 > OK for trunk?
 >
 >
 > H.J.
 > ---
 > gcc/
 >
 > PR target/85345
 > * cgraph.h: Include stringpool.h" and "attribs.h".
 > (cgraph_node::only_called_directly_or_aliased_p): Return false
 > for IFUNC resolver.
 >
 > gcc/testsuite/
 >
 > PR target/85345
 > * gcc.target/i386/pr85345.c: New test.
 > ---
 >  gcc/cgraph.h|  5 +++-
 >  gcc/testsuite/gcc.target/i386/pr85345.c | 44 
 > +
 >  2 files changed, 48 insertions(+), 1 deletion(-)
 >  create mode 100644 gcc/testsuite/gcc.target/i386/pr85345.c
 >
 > diff --git a/gcc/cgraph.h b/gcc/cgraph.h
 > index d1ef8408497..9e195824fcc 100644
 > --- a/gcc/cgraph.h
 > +++ b/gcc/cgraph.h
 > @@ -24,6 +24,8 @@ along with GCC; see the file COPYING3.  If not see
 >  #include "profile-count.h"
 >  #include "ipa-ref.h"
 >  #include "plugin-api.h"
 > +#include "stringpool.h"
 > +#include "attribs.h"
 >
 >  class ipa_opt_pass_d;
 >  typedef ipa_opt_pass_d *ipa_opt_pass;
 > @@ -2894,7 +2896,8 @@ cgraph_node::only_called_directly_or_aliased_p 
 > (void)
 >   && !DECL_STATIC_CONSTRUCTOR (decl)
 >   && !DECL_STATIC_DESTRUCTOR (decl)
 >   && !used_from_object_file_p ()
 > - && !externally_visible);
 > + && !externally_visible
 > + && !lookup_attribute ("ifunc", DECL_ATTRIBUTES (decl)));

 How's it handled for our own generated resolver functions?  That is,
 isn't there sth cheaper than doing a lookup_attribute here?  I see
 that make_dispatcher_decl nor ix86_get_function_versions_dispatcher
 adds the 'ifunc' attribute (though they are TREE_PUBLIC there).
>>>
>>> Is there any drawback of setting force_output flag?
>>> Honza
>>
>> Setting force_output may prevent some optimizations.  Can we add a bit
>> for IFUNC resolver?
>>
>
> Here is the patch to add ifunc_resolver to cgraph_node. Tested on x86-64
> and i686.  Any comments?
>

PING:

https://gcc.gnu.org/ml/gcc-patches/2018-04/msg00647.html


-- 
H.J.


Re: [PATCH] Don't mark IFUNC resolver as only called directly

2018-04-12 Thread H.J. Lu
On Thu, Apr 12, 2018 at 6:39 AM, H.J. Lu <hjl.to...@gmail.com> wrote:
> On Thu, Apr 12, 2018 at 5:17 AM, Jan Hubicka <hubi...@ucw.cz> wrote:
>>> On Thu, Apr 12, 2018 at 1:29 PM, H.J. Lu <hongjiu...@intel.com> wrote:
>>> > Since IFUNC resolver is called indirectly, don't mark IFUNC resolver as
>>> > only called directly.
>>> >
>>> > OK for trunk?
>>> >
>>> >
>>> > H.J.
>>> > ---
>>> > gcc/
>>> >
>>> > PR target/85345
>>> > * cgraph.h: Include stringpool.h" and "attribs.h".
>>> > (cgraph_node::only_called_directly_or_aliased_p): Return false
>>> > for IFUNC resolver.
>>> >
>>> > gcc/testsuite/
>>> >
>>> > PR target/85345
>>> > * gcc.target/i386/pr85345.c: New test.
>>> > ---
>>> >  gcc/cgraph.h|  5 +++-
>>> >  gcc/testsuite/gcc.target/i386/pr85345.c | 44 
>>> > +
>>> >  2 files changed, 48 insertions(+), 1 deletion(-)
>>> >  create mode 100644 gcc/testsuite/gcc.target/i386/pr85345.c
>>> >
>>> > diff --git a/gcc/cgraph.h b/gcc/cgraph.h
>>> > index d1ef8408497..9e195824fcc 100644
>>> > --- a/gcc/cgraph.h
>>> > +++ b/gcc/cgraph.h
>>> > @@ -24,6 +24,8 @@ along with GCC; see the file COPYING3.  If not see
>>> >  #include "profile-count.h"
>>> >  #include "ipa-ref.h"
>>> >  #include "plugin-api.h"
>>> > +#include "stringpool.h"
>>> > +#include "attribs.h"
>>> >
>>> >  class ipa_opt_pass_d;
>>> >  typedef ipa_opt_pass_d *ipa_opt_pass;
>>> > @@ -2894,7 +2896,8 @@ cgraph_node::only_called_directly_or_aliased_p 
>>> > (void)
>>> >   && !DECL_STATIC_CONSTRUCTOR (decl)
>>> >   && !DECL_STATIC_DESTRUCTOR (decl)
>>> >   && !used_from_object_file_p ()
>>> > - && !externally_visible);
>>> > + && !externally_visible
>>> > + && !lookup_attribute ("ifunc", DECL_ATTRIBUTES (decl)));
>>>
>>> How's it handled for our own generated resolver functions?  That is,
>>> isn't there sth cheaper than doing a lookup_attribute here?  I see
>>> that make_dispatcher_decl nor ix86_get_function_versions_dispatcher
>>> adds the 'ifunc' attribute (though they are TREE_PUBLIC there).
>>
>> Is there any drawback of setting force_output flag?
>> Honza
>
> Setting force_output may prevent some optimizations.  Can we add a bit
> for IFUNC resolver?
>

Here is the patch to add ifunc_resolver to cgraph_node. Tested on x86-64
and i686.  Any comments?

Thanks.

-- 
H.J.
From 283a3282d018a40ab550a137a5a2770ce63f4a40 Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.to...@gmail.com>
Date: Wed, 11 Apr 2018 12:31:21 -0700
Subject: [PATCH] Don't mark IFUNC resolver as only called directly

Since IFUNC resolver is called indirectly, don't mark IFUNC resolver as
only called directly.  This patch adds ifunc_resolver to cgraph_node,
sets ifunc_resolver for ifunc attribute and checks ifunc_resolver
instead of looking up ifunc attribute.

gcc/

	PR target/85345
	* cgraph.h (cgraph_node::create): Set ifunc_resolver for ifunc
	attribute.
	(cgraph_node::create_alias): Likewise.
	(cgraph_node::get_availability): Check ifunc_resolver instead
	of looking up ifunc attribute.
	* cgraphunit.c (maybe_diag_incompatible_alias): Likewise.
	* symtab.c (symtab_node::binds_to_current_def_p): Likewise.
	* varasm.c (do_assemble_alias): Likewise.
	(assemble_alias): Likewise.
	(default_binds_local_p_3): Likewise.
	* cgraph.h (cgraph_node): Add ifunc_resolver.
	(cgraph_node::only_called_directly_or_aliased_p): Return false
	for IFUNC resolver.
	* lto-cgraph.c (input_node): Set ifunc_resolver for ifunc
	attribute.

gcc/testsuite/

	PR target/85345
	* gcc.target/i386/pr85345.c: New test.
---
 gcc/cgraph.c|  7 +-
 gcc/cgraph.h|  4 +++
 gcc/cgraphunit.c|  2 +-
 gcc/lto-cgraph.c|  2 ++
 gcc/symtab.c|  4 +--
 gcc/testsuite/gcc.target/i386/pr85345.c | 44 +
 gcc/varasm.c|  8 +++---
 7 files changed, 64 insertions(+), 7 deletions(-)
 create mode 100644 gcc/testsuite/gcc.target/i386/pr85345.c

diff --git a/gcc/cgraph.c b/gcc/cgraph.c
index 9a7d54d7cee..9f3a

Re: [PATCH] Don't mark IFUNC resolver as only called directly

2018-04-12 Thread H.J. Lu
On Thu, Apr 12, 2018 at 5:17 AM, Jan Hubicka  wrote:
>> On Thu, Apr 12, 2018 at 1:29 PM, H.J. Lu  wrote:
>> > Since IFUNC resolver is called indirectly, don't mark IFUNC resolver as
>> > only called directly.
>> >
>> > OK for trunk?
>> >
>> >
>> > H.J.
>> > ---
>> > gcc/
>> >
>> > PR target/85345
>> > * cgraph.h: Include stringpool.h" and "attribs.h".
>> > (cgraph_node::only_called_directly_or_aliased_p): Return false
>> > for IFUNC resolver.
>> >
>> > gcc/testsuite/
>> >
>> > PR target/85345
>> > * gcc.target/i386/pr85345.c: New test.
>> > ---
>> >  gcc/cgraph.h|  5 +++-
>> >  gcc/testsuite/gcc.target/i386/pr85345.c | 44 
>> > +
>> >  2 files changed, 48 insertions(+), 1 deletion(-)
>> >  create mode 100644 gcc/testsuite/gcc.target/i386/pr85345.c
>> >
>> > diff --git a/gcc/cgraph.h b/gcc/cgraph.h
>> > index d1ef8408497..9e195824fcc 100644
>> > --- a/gcc/cgraph.h
>> > +++ b/gcc/cgraph.h
>> > @@ -24,6 +24,8 @@ along with GCC; see the file COPYING3.  If not see
>> >  #include "profile-count.h"
>> >  #include "ipa-ref.h"
>> >  #include "plugin-api.h"
>> > +#include "stringpool.h"
>> > +#include "attribs.h"
>> >
>> >  class ipa_opt_pass_d;
>> >  typedef ipa_opt_pass_d *ipa_opt_pass;
>> > @@ -2894,7 +2896,8 @@ cgraph_node::only_called_directly_or_aliased_p (void)
>> >   && !DECL_STATIC_CONSTRUCTOR (decl)
>> >   && !DECL_STATIC_DESTRUCTOR (decl)
>> >   && !used_from_object_file_p ()
>> > - && !externally_visible);
>> > + && !externally_visible
>> > + && !lookup_attribute ("ifunc", DECL_ATTRIBUTES (decl)));
>>
>> How's it handled for our own generated resolver functions?  That is,
>> isn't there sth cheaper than doing a lookup_attribute here?  I see
>> that make_dispatcher_decl nor ix86_get_function_versions_dispatcher
>> adds the 'ifunc' attribute (though they are TREE_PUBLIC there).
>
> Is there any drawback of setting force_output flag?
> Honza

Setting force_output may prevent some optimizations.  Can we add a bit
for IFUNC resolver?

-- 
H.J.


Re: [PATCH] Don't mark IFUNC resolver as only called directly

2018-04-12 Thread H.J. Lu
On Thu, Apr 12, 2018 at 5:13 AM, Richard Biener
 wrote:
> On Thu, Apr 12, 2018 at 1:29 PM, H.J. Lu  wrote:
>> Since IFUNC resolver is called indirectly, don't mark IFUNC resolver as
>> only called directly.
>>
>> OK for trunk?
>>
>>
>> H.J.
>> ---
>> gcc/
>>
>> PR target/85345
>> * cgraph.h: Include stringpool.h" and "attribs.h".
>> (cgraph_node::only_called_directly_or_aliased_p): Return false
>> for IFUNC resolver.
>>
>> gcc/testsuite/
>>
>> PR target/85345
>> * gcc.target/i386/pr85345.c: New test.
>> ---
>>  gcc/cgraph.h|  5 +++-
>>  gcc/testsuite/gcc.target/i386/pr85345.c | 44 
>> +
>>  2 files changed, 48 insertions(+), 1 deletion(-)
>>  create mode 100644 gcc/testsuite/gcc.target/i386/pr85345.c
>>
>> diff --git a/gcc/cgraph.h b/gcc/cgraph.h
>> index d1ef8408497..9e195824fcc 100644
>> --- a/gcc/cgraph.h
>> +++ b/gcc/cgraph.h
>> @@ -24,6 +24,8 @@ along with GCC; see the file COPYING3.  If not see
>>  #include "profile-count.h"
>>  #include "ipa-ref.h"
>>  #include "plugin-api.h"
>> +#include "stringpool.h"
>> +#include "attribs.h"
>>
>>  class ipa_opt_pass_d;
>>  typedef ipa_opt_pass_d *ipa_opt_pass;
>> @@ -2894,7 +2896,8 @@ cgraph_node::only_called_directly_or_aliased_p (void)
>>   && !DECL_STATIC_CONSTRUCTOR (decl)
>>   && !DECL_STATIC_DESTRUCTOR (decl)
>>   && !used_from_object_file_p ()
>> - && !externally_visible);
>> + && !externally_visible
>> + && !lookup_attribute ("ifunc", DECL_ATTRIBUTES (decl)));
>
> How's it handled for our own generated resolver functions?  That is,
> isn't there sth cheaper than doing a lookup_attribute here?  I see
> that make_dispatcher_decl nor ix86_get_function_versions_dispatcher
> adds the 'ifunc' attribute (though they are TREE_PUBLIC there).
>

ext/mv*.C tests failed to compile:

error: '-fcf-protection=full' requires Intel CET support. Use -mcet or
both of -mibt and -mshstk options to enable CET

with -fcf-protection -mcet.   So it is unsupported.

-- 
H.J.


Re: [PATCH] Don't mark IFUNC resolver as only called directly

2018-04-12 Thread Jan Hubicka
> On Thu, Apr 12, 2018 at 1:29 PM, H.J. Lu  wrote:
> > Since IFUNC resolver is called indirectly, don't mark IFUNC resolver as
> > only called directly.
> >
> > OK for trunk?
> >
> >
> > H.J.
> > ---
> > gcc/
> >
> > PR target/85345
> > * cgraph.h: Include stringpool.h" and "attribs.h".
> > (cgraph_node::only_called_directly_or_aliased_p): Return false
> > for IFUNC resolver.
> >
> > gcc/testsuite/
> >
> > PR target/85345
> > * gcc.target/i386/pr85345.c: New test.
> > ---
> >  gcc/cgraph.h|  5 +++-
> >  gcc/testsuite/gcc.target/i386/pr85345.c | 44 
> > +
> >  2 files changed, 48 insertions(+), 1 deletion(-)
> >  create mode 100644 gcc/testsuite/gcc.target/i386/pr85345.c
> >
> > diff --git a/gcc/cgraph.h b/gcc/cgraph.h
> > index d1ef8408497..9e195824fcc 100644
> > --- a/gcc/cgraph.h
> > +++ b/gcc/cgraph.h
> > @@ -24,6 +24,8 @@ along with GCC; see the file COPYING3.  If not see
> >  #include "profile-count.h"
> >  #include "ipa-ref.h"
> >  #include "plugin-api.h"
> > +#include "stringpool.h"
> > +#include "attribs.h"
> >
> >  class ipa_opt_pass_d;
> >  typedef ipa_opt_pass_d *ipa_opt_pass;
> > @@ -2894,7 +2896,8 @@ cgraph_node::only_called_directly_or_aliased_p (void)
> >   && !DECL_STATIC_CONSTRUCTOR (decl)
> >   && !DECL_STATIC_DESTRUCTOR (decl)
> >   && !used_from_object_file_p ()
> > - && !externally_visible);
> > + && !externally_visible
> > + && !lookup_attribute ("ifunc", DECL_ATTRIBUTES (decl)));
> 
> How's it handled for our own generated resolver functions?  That is,
> isn't there sth cheaper than doing a lookup_attribute here?  I see
> that make_dispatcher_decl nor ix86_get_function_versions_dispatcher
> adds the 'ifunc' attribute (though they are TREE_PUBLIC there).

Is there any drawback of setting force_output flag?
Honza
> 
> Richard.
> 
> >  }
> >
> >  /* Return true when function can be removed from callgraph
> > diff --git a/gcc/testsuite/gcc.target/i386/pr85345.c 
> > b/gcc/testsuite/gcc.target/i386/pr85345.c
> > new file mode 100644
> > index 000..63f771294ad
> > --- /dev/null
> > +++ b/gcc/testsuite/gcc.target/i386/pr85345.c
> > @@ -0,0 +1,44 @@
> > +/* { dg-do compile } */
> > +/* { dg-options "-O2 -fcf-protection -mcet" } */
> > +/* { dg-final { scan-assembler-times {\mendbr} 4 } } */
> > +
> > +int resolver_fn = 0;
> > +int resolved_fn = 0;
> > +
> > +static inline void
> > +do_it_right_at_runtime_A (void)
> > +{
> > +  resolved_fn++;
> > +}
> > +
> > +static inline void
> > +do_it_right_at_runtime_B (void)
> > +{
> > +  resolved_fn++;
> > +}
> > +
> > +static inline void do_it_right_at_runtime (void);
> > +
> > +void do_it_right_at_runtime (void)
> > +  __attribute__ ((ifunc ("resolve_do_it_right_at_runtime")));
> > +
> > +extern int r;
> > +static void (*resolve_do_it_right_at_runtime (void)) (void)
> > +{
> > +  resolver_fn++;
> > +
> > +  typeof(do_it_right_at_runtime) *func;
> > +  if (r & 1)
> > +func = do_it_right_at_runtime_A;
> > +  else
> > +func = do_it_right_at_runtime_B;
> > +
> > +  return (void *) func;
> > +}
> > +
> > +int
> > +main ()
> > +{
> > +  do_it_right_at_runtime ();
> > +  return 0;
> > +}
> > --
> > 2.14.3
> >


Re: [PATCH] Don't mark IFUNC resolver as only called directly

2018-04-12 Thread Richard Biener
On Thu, Apr 12, 2018 at 1:29 PM, H.J. Lu  wrote:
> Since IFUNC resolver is called indirectly, don't mark IFUNC resolver as
> only called directly.
>
> OK for trunk?
>
>
> H.J.
> ---
> gcc/
>
> PR target/85345
> * cgraph.h: Include stringpool.h" and "attribs.h".
> (cgraph_node::only_called_directly_or_aliased_p): Return false
> for IFUNC resolver.
>
> gcc/testsuite/
>
> PR target/85345
> * gcc.target/i386/pr85345.c: New test.
> ---
>  gcc/cgraph.h|  5 +++-
>  gcc/testsuite/gcc.target/i386/pr85345.c | 44 
> +
>  2 files changed, 48 insertions(+), 1 deletion(-)
>  create mode 100644 gcc/testsuite/gcc.target/i386/pr85345.c
>
> diff --git a/gcc/cgraph.h b/gcc/cgraph.h
> index d1ef8408497..9e195824fcc 100644
> --- a/gcc/cgraph.h
> +++ b/gcc/cgraph.h
> @@ -24,6 +24,8 @@ along with GCC; see the file COPYING3.  If not see
>  #include "profile-count.h"
>  #include "ipa-ref.h"
>  #include "plugin-api.h"
> +#include "stringpool.h"
> +#include "attribs.h"
>
>  class ipa_opt_pass_d;
>  typedef ipa_opt_pass_d *ipa_opt_pass;
> @@ -2894,7 +2896,8 @@ cgraph_node::only_called_directly_or_aliased_p (void)
>   && !DECL_STATIC_CONSTRUCTOR (decl)
>   && !DECL_STATIC_DESTRUCTOR (decl)
>   && !used_from_object_file_p ()
> - && !externally_visible);
> + && !externally_visible
> + && !lookup_attribute ("ifunc", DECL_ATTRIBUTES (decl)));

How's it handled for our own generated resolver functions?  That is,
isn't there sth cheaper than doing a lookup_attribute here?  I see
that make_dispatcher_decl nor ix86_get_function_versions_dispatcher
adds the 'ifunc' attribute (though they are TREE_PUBLIC there).

Richard.

>  }
>
>  /* Return true when function can be removed from callgraph
> diff --git a/gcc/testsuite/gcc.target/i386/pr85345.c 
> b/gcc/testsuite/gcc.target/i386/pr85345.c
> new file mode 100644
> index 000..63f771294ad
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/i386/pr85345.c
> @@ -0,0 +1,44 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -fcf-protection -mcet" } */
> +/* { dg-final { scan-assembler-times {\mendbr} 4 } } */
> +
> +int resolver_fn = 0;
> +int resolved_fn = 0;
> +
> +static inline void
> +do_it_right_at_runtime_A (void)
> +{
> +  resolved_fn++;
> +}
> +
> +static inline void
> +do_it_right_at_runtime_B (void)
> +{
> +  resolved_fn++;
> +}
> +
> +static inline void do_it_right_at_runtime (void);
> +
> +void do_it_right_at_runtime (void)
> +  __attribute__ ((ifunc ("resolve_do_it_right_at_runtime")));
> +
> +extern int r;
> +static void (*resolve_do_it_right_at_runtime (void)) (void)
> +{
> +  resolver_fn++;
> +
> +  typeof(do_it_right_at_runtime) *func;
> +  if (r & 1)
> +func = do_it_right_at_runtime_A;
> +  else
> +func = do_it_right_at_runtime_B;
> +
> +  return (void *) func;
> +}
> +
> +int
> +main ()
> +{
> +  do_it_right_at_runtime ();
> +  return 0;
> +}
> --
> 2.14.3
>


[PATCH] Don't mark IFUNC resolver as only called directly

2018-04-12 Thread H.J. Lu
Since IFUNC resolver is called indirectly, don't mark IFUNC resolver as
only called directly.

OK for trunk?


H.J.
---
gcc/

PR target/85345
* cgraph.h: Include stringpool.h" and "attribs.h".
(cgraph_node::only_called_directly_or_aliased_p): Return false
for IFUNC resolver.

gcc/testsuite/

PR target/85345
* gcc.target/i386/pr85345.c: New test.
---
 gcc/cgraph.h|  5 +++-
 gcc/testsuite/gcc.target/i386/pr85345.c | 44 +
 2 files changed, 48 insertions(+), 1 deletion(-)
 create mode 100644 gcc/testsuite/gcc.target/i386/pr85345.c

diff --git a/gcc/cgraph.h b/gcc/cgraph.h
index d1ef8408497..9e195824fcc 100644
--- a/gcc/cgraph.h
+++ b/gcc/cgraph.h
@@ -24,6 +24,8 @@ along with GCC; see the file COPYING3.  If not see
 #include "profile-count.h"
 #include "ipa-ref.h"
 #include "plugin-api.h"
+#include "stringpool.h"
+#include "attribs.h"
 
 class ipa_opt_pass_d;
 typedef ipa_opt_pass_d *ipa_opt_pass;
@@ -2894,7 +2896,8 @@ cgraph_node::only_called_directly_or_aliased_p (void)
  && !DECL_STATIC_CONSTRUCTOR (decl)
  && !DECL_STATIC_DESTRUCTOR (decl)
  && !used_from_object_file_p ()
- && !externally_visible);
+ && !externally_visible
+ && !lookup_attribute ("ifunc", DECL_ATTRIBUTES (decl)));
 }
 
 /* Return true when function can be removed from callgraph
diff --git a/gcc/testsuite/gcc.target/i386/pr85345.c 
b/gcc/testsuite/gcc.target/i386/pr85345.c
new file mode 100644
index 000..63f771294ad
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/pr85345.c
@@ -0,0 +1,44 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -fcf-protection -mcet" } */
+/* { dg-final { scan-assembler-times {\mendbr} 4 } } */
+
+int resolver_fn = 0;
+int resolved_fn = 0;
+
+static inline void
+do_it_right_at_runtime_A (void)
+{
+  resolved_fn++;
+}
+
+static inline void
+do_it_right_at_runtime_B (void)
+{
+  resolved_fn++;
+}
+
+static inline void do_it_right_at_runtime (void);
+
+void do_it_right_at_runtime (void)
+  __attribute__ ((ifunc ("resolve_do_it_right_at_runtime")));
+
+extern int r;
+static void (*resolve_do_it_right_at_runtime (void)) (void)
+{
+  resolver_fn++;
+
+  typeof(do_it_right_at_runtime) *func;
+  if (r & 1)
+func = do_it_right_at_runtime_A;
+  else
+func = do_it_right_at_runtime_B;
+
+  return (void *) func;
+}
+
+int
+main ()
+{
+  do_it_right_at_runtime ();
+  return 0;
+}
-- 
2.14.3