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

[H.J. Lu]

On Wed, May 23, 2018 at 8:11 AM, Jan Hubicka <hubicka@ucw.cz> wrote:
>> On Wed, May 23, 2018 at 2:01 AM, Jan Hubicka <hubicka@ucw.cz> wrote:
>> >> On Tue, May 22, 2018 at 9:21 AM, Jan Hubicka <hubicka@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 <cgraph_node *> (this)->ifunc_resolver)
>> >> +  {
>> >> +          error ("inconsistent `ifunc' attribute");
>> >> +          error_found = true;
>> >> +  }
>> >>      }
>> >>    else if (is_a <varpool_node *> (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.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Don-t-mark-IFUNC-resolver-as-only-called-directly.patch
Type: text/x-patch
Size: 8219 bytes
Desc: not available
URL: <http://gcc.gnu.org/pipermail/gcc-patches/attachments/20180523/f48a99e2/attachment.bin>