| Message ID | 20180412112925.GA222547@intel.com |
|---|---|
| State | New |
| Headers | show |
| Series | Don't mark IFUNC resolver as only called directly | expand |
On Thu, Apr 12, 2018 at 1:29 PM, H.J. Lu <hongjiu.lu@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). 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 00000000000..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 >
> On Thu, Apr 12, 2018 at 1:29 PM, H.J. Lu <hongjiu.lu@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 > > 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 00000000000..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 > >
On Thu, Apr 12, 2018 at 5:13 AM, Richard Biener <richard.guenther@gmail.com> wrote: > On Thu, Apr 12, 2018 at 1:29 PM, H.J. Lu <hongjiu.lu@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). > 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.
On Thu, Apr 12, 2018 at 5:17 AM, Jan Hubicka <hubicka@ucw.cz> wrote: >> On Thu, Apr 12, 2018 at 1:29 PM, H.J. Lu <hongjiu.lu@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?
On Thu, Apr 12, 2018 at 6:39 AM, H.J. Lu <hjl.tools@gmail.com> wrote: > On Thu, Apr 12, 2018 at 5:17 AM, Jan Hubicka <hubicka@ucw.cz> wrote: >>> On Thu, Apr 12, 2018 at 1:29 PM, H.J. Lu <hongjiu.lu@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.
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 00000000000..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; +}