diff mbox series

PR libgcc/83112, Fix warnings on libgcc float128-ifunc.c

Message ID 20171127192131.GA15914@ibm-tiger.the-meissners.org
State New
Headers show
Series PR libgcc/83112, Fix warnings on libgcc float128-ifunc.c | expand

Commit Message

Michael Meissner Nov. 27, 2017, 7:21 p.m. UTC
The new -Wattribute-alias option now issues warnings for old-style ifunc
declarations that coerce the pointer to the function to void *.  The
float128-ifunc.c module in libgcc/config/rs6000 now gets a lot of warnings of
the form:

../float128-ifunc.c:109:1: warning: ‘ifunc’ resolver for ‘__negkf2’ should
return ‘TFtype (*)(TFtype) {aka _Float128 (*)(_Float128)}’ [-Wattribute-alias]

This patch fixes these warnings.  I have done a full bootstrap build and test
suite run.  I have verified that the ifunc handler works correctly, using
software emulation on a power8 and the hardware instructions on power9.  Can I
check this into the trunk?

2017-11-27  Michael Meissner  <meissner@linux.vnet.ibm.com>

	PR libgcc/83112
	* config/rs6000/float128-ifunc.c (__addkf3_resolve): Use the
	correct type for all ifunc resolvers to silence -Wattribute-alias
	warnings.
	(__subkf3_resolve): Likewise.
	(__mulkf3_resolve): Likewise.
	(__divkf3_resolve): Likewise.
	(__negkf2_resolve): Likewise.
	(__eqkf2_resolve): Likewise.
	(__nekf2_resolve): Likewise.
	(__gekf2_resolve): Likewise.
	(__gtkf2_resolve): Likewise.
	(__lekf2_resolve): Likewise.
	(__ltkf2_resolve): Likewise.
	(__unordkf2_resolve): Likewise.
	(__extendsfkf2_resolve): Likewise.
	(__extenddfkf2_resolve): Likewise.
	(__trunckfsf2_resolve): Likewise.
	(__trunckfdf2_resolve): Likewise.
	(__fixkfsi_resolve): Likewise.
	(__fixkfdi_resolve): Likewise.
	(__fixunskfsi_resolve): Likewise.
	(__fixunskfdi_resolve): Likewise.
	(__floatsikf_resolve): Likewise.
	(__floatdikf_resolve): Likewise.
	(__floatunsikf_resolve): Likewise.
	(__floatundikf_resolve): Likewise.
	(__extendkftf2_resolve): Likewise.
	(__trunctfkf2_resolve): Likewise.

Comments

Martin Sebor Nov. 27, 2017, 10:40 p.m. UTC | #1
On 11/27/2017 12:21 PM, Michael Meissner wrote:
> The new -Wattribute-alias option now issues warnings for old-style ifunc
> declarations that coerce the pointer to the function to void *.  The
> float128-ifunc.c module in libgcc/config/rs6000 now gets a lot of warnings of
> the form:
>
> ../float128-ifunc.c:109:1: warning: ‘ifunc’ resolver for ‘__negkf2’ should
> return ‘TFtype (*)(TFtype) {aka _Float128 (*)(_Float128)}’ [-Wattribute-alias]
>
> This patch fixes these warnings.  I have done a full bootstrap build and test
> suite run.  I have verified that the ifunc handler works correctly, using
> software emulation on a power8 and the hardware instructions on power9.  Can I
> check this into the trunk?

Just as a side note, a convenient way to deal with this is to use
typeof to deduce the return type of the resolver from the type of
the function it returns.  I would expect something like
the following untested change to do it and make the typedefs
unnecessary:

   -static void *
   +static __typeof__ (__addkf3_sw) *
    __addkf3_resolve (void)
    {
   -  return (void *) SW_OR_HW (__addkf3_sw, __addkf3_hw);
   +  return SW_OR_HW (__addkf3_sw, __addkf3_hw);
    }

Martin
Michael Meissner Nov. 27, 2017, 11:40 p.m. UTC | #2
On Mon, Nov 27, 2017 at 03:40:58PM -0700, Martin Sebor wrote:
> On 11/27/2017 12:21 PM, Michael Meissner wrote:
> >The new -Wattribute-alias option now issues warnings for old-style ifunc
> >declarations that coerce the pointer to the function to void *.  The
> >float128-ifunc.c module in libgcc/config/rs6000 now gets a lot of warnings of
> >the form:
> >
> >../float128-ifunc.c:109:1: warning: ‘ifunc’ resolver for ‘__negkf2’ should
> >return ‘TFtype (*)(TFtype) {aka _Float128 (*)(_Float128)}’ [-Wattribute-alias]
> >
> >This patch fixes these warnings.  I have done a full bootstrap build and test
> >suite run.  I have verified that the ifunc handler works correctly, using
> >software emulation on a power8 and the hardware instructions on power9.  Can I
> >check this into the trunk?
> 
> Just as a side note, a convenient way to deal with this is to use
> typeof to deduce the return type of the resolver from the type of
> the function it returns.  I would expect something like
> the following untested change to do it and make the typedefs
> unnecessary:
> 
>   -static void *
>   +static __typeof__ (__addkf3_sw) *
>    __addkf3_resolve (void)
>    {
>   -  return (void *) SW_OR_HW (__addkf3_sw, __addkf3_hw);
>   +  return SW_OR_HW (__addkf3_sw, __addkf3_hw);
>    }
> 
> Martin

Thanks, that does simplify things.  I redid the patch for both PR libgcc/83112
(avoid the warnings in float128-ifunc) and PR libgcc/83103 (optimize complex
float128 multiply/divide when running on ISA 3.0 hardware).

This patch combines both PRs.  I have bootstrapped it and done a regression
test with no regressions.  Can I install this patch in the trunk?

2017-11-27  Michael Meissner  <meissner@linux.vnet.ibm.com>

	PR libgcc/83112
	* config/rs6000/float128-ifunc.c (__addkf3_resolve): Use the
	correct type for all ifunc resolvers to silence -Wattribute-alias
	warnings.  Eliminate the forward declaration of the resolver
	functions which is no longer needed.
	(__subkf3_resolve): Likewise.
	(__mulkf3_resolve): Likewise.
	(__divkf3_resolve): Likewise.
	(__negkf2_resolve): Likewise.
	(__eqkf2_resolve): Likewise.
	(__nekf2_resolve): Likewise.
	(__gekf2_resolve): Likewise.
	(__gtkf2_resolve): Likewise.
	(__lekf2_resolve): Likewise.
	(__ltkf2_resolve): Likewise.
	(__unordkf2_resolve): Likewise.
	(__extendsfkf2_resolve): Likewise.
	(__extenddfkf2_resolve): Likewise.
	(__trunckfsf2_resolve): Likewise.
	(__trunckfdf2_resolve): Likewise.
	(__fixkfsi_resolve): Likewise.
	(__fixkfdi_resolve): Likewise.
	(__fixunskfsi_resolve): Likewise.
	(__fixunskfdi_resolve): Likewise.
	(__floatsikf_resolve): Likewise.
	(__floatdikf_resolve): Likewise.
	(__floatunsikf_resolve): Likewise.
	(__floatundikf_resolve): Likewise.
	(__extendkftf2_resolve): Likewise.
	(__trunctfkf2_resolve): Likewise.

	PR libgcc/83103
	* config/rs6000/quad-float128.h (TF): Don't define if long double
	is IEEE 128-bit floating point.
	(TCtype): Define as either TCmode or KCmode, depending on whether
	long double is IEEE 128-bit floating point.
	(__mulkc3_sw): Add declarations for software/hardware versions of
	complex multiply/divide.
	(__divkc3_sw): Likewise.
	(__mulkc3_hw): Likewise.
	(__divkc3_hw): Likewise.
	* config/rs6000/_mulkc3.c (_mulkc3): If we are building ifunc
	handlers to switch between using software emulation and hardware
	float128 instructions, build the complex multiply/divide functions
	for both software and hardware support.
	* config/rs6000/_divkc3.c (_divkc3): Likewise.
	* config/rs6000/float128-ifunc.c (__mulkc3_resolve): Likewise.
	(__divkc3_resolve): Likewise.
	(__mulkc3): Likewise.
	(__divkc3): Likewise.
	* config/rs6000/t-float128-hw (fp128_hardfp_src): Likewise.
	(fp128_hw_src): Likewise.
	(fp128_hw_static_obj): Likewise.
	(fp128_hw_shared_obj): Likewise.
	(_mulkc3-hw.c): Likewise.
	(_divkc3-hw.c): Likewise.
	* config/rs6000/t-float128 (clean-float128): Delete _mulkc3-hw.c
	and _divkc3-hw.c.
Segher Boessenkool Nov. 30, 2017, 2:42 a.m. UTC | #3
Hi,

On Mon, Nov 27, 2017 at 06:40:09PM -0500, Michael Meissner wrote:
> @@ -33,3 +35,13 @@ $(fp128_hw_obj)		 : $(srcdir)/config/rs6
>  
>  $(fp128_ifunc_obj)	 : INTERNAL_CFLAGS += $(FP128_CFLAGS_SW)
>  $(fp128_ifunc_obj)	 : $(srcdir)/config/rs6000/t-float128-hw
> +
> +_mulkc3-hw.c: $(srcdir)/config/rs6000/_mulkc3.c
> +	rm -rf _mulkc3.c
> +	(echo "#define __mulkc3 __mulkc3_hw"; \
> +	 cat $(srcdir)/config/rs6000/_mulkc3.c) > _mulkc3-hw.c

Please don't -rf.  -rf is a dangerous habit.

This also won't work if anything tries to build _mulkc3-hw.c a second
time; you have deleted its prerequisite.

Maybe some other scheme is better?

> --- libgcc/config/rs6000/t-float128	(revision 255177)
> +++ libgcc/config/rs6000/t-float128	(working copy)
> @@ -86,7 +86,7 @@ test:
>  	for x in $(fp128_obj); do echo "    $$x"; done;
>  
>  clean-float128:
> -	rm -rf $(fp128_softfp_src)
> +	rm -rf $(fp128_softfp_src) $(fp128_hardfp_src)
>  	@$(MULTICLEAN) multi-clean DO=clean-float128

-rm to avoid warnings from rm if you clean without the files being there.

Otherwise looks good.  Thanks!


Segher
Michael Meissner Nov. 30, 2017, 8:54 p.m. UTC | #4
On Wed, Nov 29, 2017 at 08:42:51PM -0600, Segher Boessenkool wrote:
> Hi,
> 
> On Mon, Nov 27, 2017 at 06:40:09PM -0500, Michael Meissner wrote:
> > @@ -33,3 +35,13 @@ $(fp128_hw_obj)		 : $(srcdir)/config/rs6
> >  
> >  $(fp128_ifunc_obj)	 : INTERNAL_CFLAGS += $(FP128_CFLAGS_SW)
> >  $(fp128_ifunc_obj)	 : $(srcdir)/config/rs6000/t-float128-hw
> > +
> > +_mulkc3-hw.c: $(srcdir)/config/rs6000/_mulkc3.c
> > +	rm -rf _mulkc3.c
> > +	(echo "#define __mulkc3 __mulkc3_hw"; \
> > +	 cat $(srcdir)/config/rs6000/_mulkc3.c) > _mulkc3-hw.c
> 
> Please don't -rf.  -rf is a dangerous habit.
> 
> This also won't work if anything tries to build _mulkc3-hw.c a second
> time; you have deleted its prerequisite.
> 
> Maybe some other scheme is better?
> 
> > --- libgcc/config/rs6000/t-float128	(revision 255177)
> > +++ libgcc/config/rs6000/t-float128	(working copy)
> > @@ -86,7 +86,7 @@ test:
> >  	for x in $(fp128_obj); do echo "    $$x"; done;
> >  
> >  clean-float128:
> > -	rm -rf $(fp128_softfp_src)
> > +	rm -rf $(fp128_softfp_src) $(fp128_hardfp_src)
> >  	@$(MULTICLEAN) multi-clean DO=clean-float128
> 
> -rm to avoid warnings from rm if you clean without the files being there.
> 
> Otherwise looks good.  Thanks!

As we discussed on private IRC, I mistakenly deleted the _{mul,div}kc3.c file
when I meant to delete the _{mul,div}kc3-hw.c file before recreating it.

This is the patch I checked in (subversion id 255282):

2017-11-30  Michael Meissner  <meissner@linux.vnet.ibm.com>

	PR libgcc/83112
	* config/rs6000/float128-ifunc.c (__addkf3_resolve): Use the
	correct type for all ifunc resolvers to silence -Wattribute-alias
	warnings.  Eliminate the forward declaration of the resolver
	functions which is no longer needed.
	(__subkf3_resolve): Likewise.
	(__mulkf3_resolve): Likewise.
	(__divkf3_resolve): Likewise.
	(__negkf2_resolve): Likewise.
	(__eqkf2_resolve): Likewise.
	(__nekf2_resolve): Likewise.
	(__gekf2_resolve): Likewise.
	(__gtkf2_resolve): Likewise.
	(__lekf2_resolve): Likewise.
	(__ltkf2_resolve): Likewise.
	(__unordkf2_resolve): Likewise.
	(__extendsfkf2_resolve): Likewise.
	(__extenddfkf2_resolve): Likewise.
	(__trunckfsf2_resolve): Likewise.
	(__trunckfdf2_resolve): Likewise.
	(__fixkfsi_resolve): Likewise.
	(__fixkfdi_resolve): Likewise.
	(__fixunskfsi_resolve): Likewise.
	(__fixunskfdi_resolve): Likewise.
	(__floatsikf_resolve): Likewise.
	(__floatdikf_resolve): Likewise.
	(__floatunsikf_resolve): Likewise.
	(__floatundikf_resolve): Likewise.
	(__extendkftf2_resolve): Likewise.
	(__trunctfkf2_resolve): Likewise.

	PR libgcc/83103
	* config/rs6000/quad-float128.h (TF): Don't define if long double
	is IEEE 128-bit floating point.
	(TCtype): Define as either TCmode or KCmode, depending on whether
	long double is IEEE 128-bit floating point.
	(__mulkc3_sw): Add declarations for software/hardware versions of
	complex multiply/divide.
	(__divkc3_sw): Likewise.
	(__mulkc3_hw): Likewise.
	(__divkc3_hw): Likewise.
	* config/rs6000/_mulkc3.c (_mulkc3): If we are building ifunc
	handlers to switch between using software emulation and hardware
	float128 instructions, build the complex multiply/divide functions
	for both software and hardware support.
	* config/rs6000/_divkc3.c (_divkc3): Likewise.
	* config/rs6000/float128-ifunc.c (__mulkc3_resolve): Likewise.
	(__divkc3_resolve): Likewise.
	(__mulkc3): Likewise.
	(__divkc3): Likewise.
	* config/rs6000/t-float128-hw (fp128_hardfp_src): Likewise.
	(fp128_hw_src): Likewise.
	(fp128_hw_static_obj): Likewise.
	(fp128_hw_shared_obj): Likewise.
	(_mulkc3-hw.c): Likewise.
	(_divkc3-hw.c): Likewise.
	* config/rs6000/t-float128 (clean-float128): Delete _mulkc3-hw.c
	and _divkc3-hw.c.
Michael Meissner Dec. 1, 2017, 5:40 a.m. UTC | #5
After committing the previous patch, I noticed that it was now generating
warnings for __{mul,div}kc3_{sw,hw} not having a prototype that I hadn't
noticed during development of the patch.  This is due to the fact that before I
added the ifunc support, it was only compiling __{mul,div}kc3, and those have
built-in declarations.  I installed this patch as being obvious:

2017-11-30  Michael Meissner  <meissner@linux.vnet.ibm.com>

	* config/rs6000/_mulkc3.c (__mulkc3): Add forward declaration.
	* config/rs6000/_divkc3.c (__divkc3): Likewise.

Index: libgcc/config/rs6000/_divkc3.c
===================================================================
--- libgcc/config/rs6000/_divkc3.c	(revision 255288)
+++ libgcc/config/rs6000/_divkc3.c	(working copy)
@@ -37,6 +37,8 @@ typedef __complex float KCtype __attribu
 #define __divkc3 __divkc3_sw
 #endif
 
+extern KCtype __divkc3 (KFtype, KFtype, KFtype, KFtype);
+
 KCtype
 __divkc3 (KFtype a, KFtype b, KFtype c, KFtype d)
 {
Index: libgcc/config/rs6000/_mulkc3.c
===================================================================
--- libgcc/config/rs6000/_mulkc3.c	(revision 255288)
+++ libgcc/config/rs6000/_mulkc3.c	(working copy)
@@ -35,6 +35,8 @@ typedef __complex float KCtype __attribu
 #define __mulkc3 __mulkc3_sw
 #endif
 
+extern KCtype __mulkc3 (KFtype, KFtype, KFtype, KFtype);
+
 KCtype
 __mulkc3 (KFtype a, KFtype b, KFtype c, KFtype d)
 {
Segher Boessenkool Dec. 1, 2017, 11:53 p.m. UTC | #6
On Fri, Dec 01, 2017 at 12:40:22AM -0500, Michael Meissner wrote:
> After committing the previous patch, I noticed that it was now generating
> warnings for __{mul,div}kc3_{sw,hw} not having a prototype that I hadn't
> noticed during development of the patch.  This is due to the fact that before I
> added the ifunc support, it was only compiling __{mul,div}kc3, and those have
> built-in declarations.  I installed this patch as being obvious:
> 
> 2017-11-30  Michael Meissner  <meissner@linux.vnet.ibm.com>
> 
> 	* config/rs6000/_mulkc3.c (__mulkc3): Add forward declaration.
> 	* config/rs6000/_divkc3.c (__divkc3): Likewise.
> 
> Index: libgcc/config/rs6000/_divkc3.c
> ===================================================================
> --- libgcc/config/rs6000/_divkc3.c	(revision 255288)
> +++ libgcc/config/rs6000/_divkc3.c	(working copy)
> @@ -37,6 +37,8 @@ typedef __complex float KCtype __attribu
>  #define __divkc3 __divkc3_sw
>  #endif
>  
> +extern KCtype __divkc3 (KFtype, KFtype, KFtype, KFtype);
> +
>  KCtype
>  __divkc3 (KFtype a, KFtype b, KFtype c, KFtype d)
>  {

How does this warn?  -Wmissing-declarations?  Should this declaration be
in a header then?

A code comment explaining why you do a declaration for exactly the same
thing as there is two lines later would help; otherwise people will try
to delete it again :-)


Segher
Michael Meissner Dec. 11, 2017, 8:57 p.m. UTC | #7
On Fri, Dec 01, 2017 at 05:53:55PM -0600, Segher Boessenkool wrote:
> On Fri, Dec 01, 2017 at 12:40:22AM -0500, Michael Meissner wrote:
> > After committing the previous patch, I noticed that it was now generating
> > warnings for __{mul,div}kc3_{sw,hw} not having a prototype that I hadn't
> > noticed during development of the patch.  This is due to the fact that before I
> > added the ifunc support, it was only compiling __{mul,div}kc3, and those have
> > built-in declarations.  I installed this patch as being obvious:
> > 
> > 2017-11-30  Michael Meissner  <meissner@linux.vnet.ibm.com>
> > 
> > 	* config/rs6000/_mulkc3.c (__mulkc3): Add forward declaration.
> > 	* config/rs6000/_divkc3.c (__divkc3): Likewise.
> > 
> > Index: libgcc/config/rs6000/_divkc3.c
> > ===================================================================
> > --- libgcc/config/rs6000/_divkc3.c	(revision 255288)
> > +++ libgcc/config/rs6000/_divkc3.c	(working copy)
> > @@ -37,6 +37,8 @@ typedef __complex float KCtype __attribu
> >  #define __divkc3 __divkc3_sw
> >  #endif
> >  
> > +extern KCtype __divkc3 (KFtype, KFtype, KFtype, KFtype);
> > +
> >  KCtype
> >  __divkc3 (KFtype a, KFtype b, KFtype c, KFtype d)
> >  {
> 
> How does this warn?  -Wmissing-declarations?  Should this declaration be
> in a header then?

The compiler creates the call to __mulkc3 and __divkc3, and internally it has
the appropriate prototype like it does for all built-in functions (in this
case, returning an _Float128 _Complex type, and taking 4 _Float128 arguments).

So before adding ifunc support, we never noticed it didn't have a prototype,
because the compiler already has a prototype.

With ifunc support, we now need to create two separate functions, __mulkc3_sw
and __mulkc3_hw, and make __multkc3 the ifunc resolver.

So there really isn't an include file that is appropriate to put the
definitions in.  I could change it to use the soft-fp includes (including
quadmath-float128.h) if desired.

Did you want me to do that?

> A code comment explaining why you do a declaration for exactly the same
> thing as there is two lines later would help; otherwise people will try
> to delete it again :-)
Segher Boessenkool Dec. 12, 2017, 5:04 p.m. UTC | #8
On Mon, Dec 11, 2017 at 03:57:51PM -0500, Michael Meissner wrote:
> > > +extern KCtype __divkc3 (KFtype, KFtype, KFtype, KFtype);
> > > +
> > >  KCtype
> > >  __divkc3 (KFtype a, KFtype b, KFtype c, KFtype d)
> > >  {
> > 
> > How does this warn?  -Wmissing-declarations?  Should this declaration be
> > in a header then?
> 
> The compiler creates the call to __mulkc3 and __divkc3, and internally it has
> the appropriate prototype like it does for all built-in functions (in this
> case, returning an _Float128 _Complex type, and taking 4 _Float128 arguments).
> 
> So before adding ifunc support, we never noticed it didn't have a prototype,
> because the compiler already has a prototype.

I still don't get it.  A function definition is also a declaration.

Something very non-intuitive is happening?

What does the patch change here?

> With ifunc support, we now need to create two separate functions, __mulkc3_sw
> and __mulkc3_hw, and make __multkc3 the ifunc resolver.
> 
> So there really isn't an include file that is appropriate to put the
> definitions in.  I could change it to use the soft-fp includes (including
> quadmath-float128.h) if desired.
> 
> Did you want me to do that?

I don't see the point in adding a second declaration right before the
existing declaration (the function definition).  I'm fine with what file
it is in.

> > A code comment explaining why you do a declaration for exactly the same
> > thing as there is two lines later would help; otherwise people will try
> > to delete it again :-)


Segher
Andreas Schwab Dec. 12, 2017, 5:18 p.m. UTC | #9
On Dez 12 2017, Segher Boessenkool <segher@kernel.crashing.org> wrote:

> On Mon, Dec 11, 2017 at 03:57:51PM -0500, Michael Meissner wrote:
>> > > +extern KCtype __divkc3 (KFtype, KFtype, KFtype, KFtype);
>> > > +
>> > >  KCtype
>> > >  __divkc3 (KFtype a, KFtype b, KFtype c, KFtype d)
>> > >  {
>> > 
>> > How does this warn?  -Wmissing-declarations?  Should this declaration be
>> > in a header then?
>> 
>> The compiler creates the call to __mulkc3 and __divkc3, and internally it has
>> the appropriate prototype like it does for all built-in functions (in this
>> case, returning an _Float128 _Complex type, and taking 4 _Float128 arguments).
>> 
>> So before adding ifunc support, we never noticed it didn't have a prototype,
>> because the compiler already has a prototype.
>
> I still don't get it.  A function definition is also a declaration.
>
> Something very non-intuitive is happening?

`-Wmissing-prototypes (C and Objective-C only)'
     Warn if a global function is defined without a previous prototype
     declaration.  This warning is issued even if the definition itself
     provides a prototype.

Andreas.
Michael Meissner Dec. 12, 2017, 9:56 p.m. UTC | #10
On Tue, Dec 12, 2017 at 11:04:55AM -0600, Segher Boessenkool wrote:
> On Mon, Dec 11, 2017 at 03:57:51PM -0500, Michael Meissner wrote:
> > > > +extern KCtype __divkc3 (KFtype, KFtype, KFtype, KFtype);
> > > > +
> > > >  KCtype
> > > >  __divkc3 (KFtype a, KFtype b, KFtype c, KFtype d)
> > > >  {
> > > 
> > > How does this warn?  -Wmissing-declarations?  Should this declaration be
> > > in a header then?
> > 
> > The compiler creates the call to __mulkc3 and __divkc3, and internally it has
> > the appropriate prototype like it does for all built-in functions (in this
> > case, returning an _Float128 _Complex type, and taking 4 _Float128 arguments).
> > 
> > So before adding ifunc support, we never noticed it didn't have a prototype,
> > because the compiler already has a prototype.
> 
> I still don't get it.  A function definition is also a declaration.
> 
> Something very non-intuitive is happening?

GCC has the following function declarations built-in:

    extern _Float128 _Complex __mulkc3 (_Float128, _Float128, _Float128, _Float128);
    extern _Float128 _Complex __divkc3 (_Float128, _Float128, _Float128, _Float128);

Before the patch, _mulkc3.c looked like:

    _Float128 _Complex
    __mulkc3 (_Float128 a, _Float128 b, _Float128 c, _Float128 d)
    {
	// ...
    }

Now, with ifunc handling it gets compiled in three separate files:

First in a file compiled with -mno-float128-hardware:

    _Float128 _Complex
    __mulkc3_sw (_Float128 a, _Float128 b, _Float128 c, _Float128 d)
    {
	// ...
    }

Second in a file compiled with -mfloat128-hardware:

    _Float128 _Complex
    __mulkc3_hw (_Float128 a, _Float128 b, _Float128 c, _Float128 d)
    {
	// ...
    }

And third as the ifunc handler:

    #define SW_OR_HW(SW, HW) (__builtin_cpu_supports ("ieee128") ? HW : SW)

    static __typeof__ (__mulkc3_sw) *
    __mulkc3_resolve (void)
    {
      return SW_OR_HW (__mulkc3_sw, __mulkc3_hw);
    }

    _Float128 _Complex __mulkc3 (Float128, _Float128, _Float128, _Float128)
	__attribute__ ((__ifunc__ ("__mulkc3_resolve")));

As Andreas points out, the option -Wmissing-prototypes complains if a global
function is compliled without prototypes for C/Objective C.

Before the patch, the internal definition within the compiler meant that that
__mulkc3 would not get the warning.  Now with separate ifunc handlers, both
__mulkc3_sw and __mulkc3_hw got warnings.
Segher Boessenkool Dec. 13, 2017, 8:57 p.m. UTC | #11
On Tue, Dec 12, 2017 at 04:56:36PM -0500, Michael Meissner wrote:
> On Tue, Dec 12, 2017 at 11:04:55AM -0600, Segher Boessenkool wrote:
> > On Mon, Dec 11, 2017 at 03:57:51PM -0500, Michael Meissner wrote:
> > > > > +extern KCtype __divkc3 (KFtype, KFtype, KFtype, KFtype);
> > > > > +
> > > > >  KCtype
> > > > >  __divkc3 (KFtype a, KFtype b, KFtype c, KFtype d)
> > > > >  {
> > > > 
> > > > How does this warn?  -Wmissing-declarations?  Should this declaration be
> > > > in a header then?

> As Andreas points out, the option -Wmissing-prototypes complains if a global
> function is compliled without prototypes for C/Objective C.
> 
> Before the patch, the internal definition within the compiler meant that that
> __mulkc3 would not get the warning.  Now with separate ifunc handlers, both
> __mulkc3_sw and __mulkc3_hw got warnings.

Gotcha.  There isn't a nice header file for it, so sure that is fine
the way you have it.  Thanks for the explanation!


Segher
diff mbox series

Patch

Index: libgcc/config/rs6000/float128-ifunc.c
===================================================================
--- libgcc/config/rs6000/float128-ifunc.c	(revision 255033)
+++ libgcc/config/rs6000/float128-ifunc.c	(working copy)
@@ -54,190 +54,208 @@ 
    128-bit integer types and 128-bit IEEE floating point, or vice versa.  So
    use the emulator functions for these conversions.  */
 
-static void *__addkf3_resolve (void);
-static void *__subkf3_resolve (void);
-static void *__mulkf3_resolve (void);
-static void *__divkf3_resolve (void);
-static void *__negkf2_resolve (void);
-static void *__eqkf2_resolve (void);
-static void *__nekf2_resolve (void);
-static void *__gekf2_resolve (void);
-static void *__gtkf2_resolve (void);
-static void *__lekf2_resolve (void);
-static void *__ltkf2_resolve (void);
-static void *__unordkf2_resolve (void);
-static void *__extendsfkf2_resolve (void);
-static void *__extenddfkf2_resolve (void);
-static void *__trunckfsf2_resolve (void);
-static void *__trunckfdf2_resolve (void);
-static void *__fixkfsi_resolve (void);
-static void *__fixkfdi_resolve (void);
-static void *__fixunskfsi_resolve (void);
-static void *__fixunskfdi_resolve (void);
-static void *__floatsikf_resolve (void);
-static void *__floatdikf_resolve (void);
-static void *__floatunsikf_resolve (void);
-static void *__floatundikf_resolve (void);
-static void *__extendkftf2_resolve (void);
-static void *__trunctfkf2_resolve (void);
+typedef TFtype (f128_func_f128_t)(TFtype);
+typedef TFtype (f128_func_f128_f128_t)(TFtype, TFtype);
+typedef CMPtype (cmp_func_f128_f128_t)(TFtype, TFtype); 
+typedef TFtype (f128_func_float_t)(float);
+typedef TFtype (f128_func_double_t)(double);
+typedef float (float_func_f128_t)(TFtype);
+typedef double (double_func_f128_t)(TFtype);
+typedef SItype_ppc (si_func_f128_t)(TFtype);
+typedef DItype_ppc (di_func_f128_t)(TFtype);
+typedef USItype_ppc (usi_func_f128_t)(TFtype);
+typedef UDItype_ppc (udi_func_f128_t)(TFtype);
+typedef TFtype (f128_func_si_t)(SItype_ppc);
+typedef TFtype (f128_func_di_t)(DItype_ppc);
+typedef TFtype (f128_func_usi_t)(USItype_ppc);
+typedef TFtype (f128_func_udi_t)(UDItype_ppc);
+typedef IBM128_TYPE (ibm_func_f128_t)(TFtype);
+typedef TFtype (f128_func_ibm_t)(IBM128_TYPE);
+
+static f128_func_f128_f128_t *__addkf3_resolve (void);
+static f128_func_f128_f128_t *__subkf3_resolve (void);
+static f128_func_f128_f128_t *__mulkf3_resolve (void);
+static f128_func_f128_f128_t *__divkf3_resolve (void);
+static f128_func_f128_t *__negkf2_resolve (void);
+static cmp_func_f128_f128_t *__eqkf2_resolve (void);
+static cmp_func_f128_f128_t *__nekf2_resolve (void);
+static cmp_func_f128_f128_t *__gekf2_resolve (void);
+static cmp_func_f128_f128_t *__gtkf2_resolve (void);
+static cmp_func_f128_f128_t *__lekf2_resolve (void);
+static cmp_func_f128_f128_t *__ltkf2_resolve (void);
+static cmp_func_f128_f128_t *__unordkf2_resolve (void);
+static f128_func_float_t *__extendsfkf2_resolve (void);
+static f128_func_double_t *__extenddfkf2_resolve (void);
+static float_func_f128_t *__trunckfsf2_resolve (void);
+static double_func_f128_t *__trunckfdf2_resolve (void);
+static si_func_f128_t *__fixkfsi_resolve (void);
+static di_func_f128_t *__fixkfdi_resolve (void);
+static usi_func_f128_t *__fixunskfsi_resolve (void);
+static udi_func_f128_t *__fixunskfdi_resolve (void);
+static f128_func_si_t *__floatsikf_resolve (void);
+static f128_func_di_t *__floatdikf_resolve (void);
+static f128_func_usi_t *__floatunsikf_resolve (void);
+static f128_func_udi_t *__floatundikf_resolve (void);
+static ibm_func_f128_t *__extendkftf2_resolve (void);
+static f128_func_ibm_t *__trunctfkf2_resolve (void);
 
-static void *
+static f128_func_f128_f128_t *
 __addkf3_resolve (void)
 {
-  return (void *) SW_OR_HW (__addkf3_sw, __addkf3_hw);
+  return SW_OR_HW (__addkf3_sw, __addkf3_hw);
 }
 
-static void *
+static f128_func_f128_f128_t *
 __subkf3_resolve (void)
 {
-  return (void *) SW_OR_HW (__subkf3_sw, __subkf3_hw);
+  return SW_OR_HW (__subkf3_sw, __subkf3_hw);
 }
 
-static void *
+static f128_func_f128_f128_t *
 __mulkf3_resolve (void)
 {
-  return (void *) SW_OR_HW (__mulkf3_sw, __mulkf3_hw);
+  return SW_OR_HW (__mulkf3_sw, __mulkf3_hw);
 }
 
-static void *
+static f128_func_f128_f128_t *
 __divkf3_resolve (void)
 {
-  return (void *) SW_OR_HW (__divkf3_sw, __divkf3_hw);
+  return SW_OR_HW (__divkf3_sw, __divkf3_hw);
 }
 
-static void *
+static f128_func_f128_t *
 __negkf2_resolve (void)
 {
-  return (void *) SW_OR_HW (__negkf2_sw, __negkf2_hw);
+  return SW_OR_HW (__negkf2_sw, __negkf2_hw);
 }
 
-static void *
+static f128_func_si_t *
 __floatsikf_resolve (void)
 {
-  return (void *) SW_OR_HW (__floatsikf_sw, __floatsikf_hw);
+  return SW_OR_HW (__floatsikf_sw, __floatsikf_hw);
 }
 
-static void *
+static f128_func_di_t *
 __floatdikf_resolve (void)
 {
-  return (void *) SW_OR_HW (__floatdikf_sw, __floatdikf_hw);
+  return SW_OR_HW (__floatdikf_sw, __floatdikf_hw);
 }
 
-static void *
+static f128_func_usi_t *
 __floatunsikf_resolve (void)
 {
-  return (void *) SW_OR_HW (__floatunsikf_sw, __floatunsikf_hw);
+  return SW_OR_HW (__floatunsikf_sw, __floatunsikf_hw);
 }
 
-static void *
+static f128_func_udi_t *
 __floatundikf_resolve (void)
 {
-  return (void *) SW_OR_HW (__floatundikf_sw, __floatundikf_hw);
+  return SW_OR_HW (__floatundikf_sw, __floatundikf_hw);
 }
 
-static void *
+static si_func_f128_t *
 __fixkfsi_resolve (void)
 {
-  return (void *) SW_OR_HW (__fixkfsi_sw, __fixkfsi_hw);
+  return SW_OR_HW (__fixkfsi_sw, __fixkfsi_hw);
 }
 
-static void *
+static di_func_f128_t *
 __fixkfdi_resolve (void)
 {
-  return (void *) SW_OR_HW (__fixkfdi_sw, __fixkfdi_hw);
+  return SW_OR_HW (__fixkfdi_sw, __fixkfdi_hw);
 }
 
-static void *
+static usi_func_f128_t *
 __fixunskfsi_resolve (void)
 {
-  return (void *) SW_OR_HW (__fixunskfsi_sw, __fixunskfsi_hw);
+  return SW_OR_HW (__fixunskfsi_sw, __fixunskfsi_hw);
 }
 
-static void *
+static udi_func_f128_t *
 __fixunskfdi_resolve (void)
 {
-  return (void *) SW_OR_HW (__fixunskfdi_sw, __fixunskfdi_hw);
+  return SW_OR_HW (__fixunskfdi_sw, __fixunskfdi_hw);
 }
 
-static void *
+static f128_func_float_t *
 __extendsfkf2_resolve (void)
 {
-  return (void *) SW_OR_HW (__extendsfkf2_sw, __extendsfkf2_hw);
+  return SW_OR_HW (__extendsfkf2_sw, __extendsfkf2_hw);
 }
 
-static void *
+static f128_func_double_t *
 __extenddfkf2_resolve (void)
 {
-  return (void *) SW_OR_HW (__extenddfkf2_sw, __extenddfkf2_hw);
+  return SW_OR_HW (__extenddfkf2_sw, __extenddfkf2_hw);
 }
 
-static void *
+static float_func_f128_t *
 __trunckfsf2_resolve (void)
 {
-  return (void *) SW_OR_HW (__trunckfsf2_sw, __trunckfsf2_hw);
+  return SW_OR_HW (__trunckfsf2_sw, __trunckfsf2_hw);
 }
 
-static void *
+static double_func_f128_t *
 __trunckfdf2_resolve (void)
 {
   return (void *) SW_OR_HW (__trunckfdf2_sw, __trunckfdf2_hw);
 }
 
-static void *
+static ibm_func_f128_t *
 __extendkftf2_resolve (void)
 {
-  return (void *) SW_OR_HW (__extendkftf2_sw, __extendkftf2_hw);
+  return SW_OR_HW (__extendkftf2_sw, __extendkftf2_hw);
 }
 
-static void *
+static f128_func_ibm_t *
 __trunctfkf2_resolve (void)
 {
   return (void *) SW_OR_HW (__trunctfkf2_sw, __trunctfkf2_hw);
 }
 
-static void *
+static cmp_func_f128_f128_t *
 __eqkf2_resolve (void)
 {
-  return (void *) SW_OR_HW (__eqkf2_sw, __eqkf2_hw);
+  return SW_OR_HW (__eqkf2_sw, __eqkf2_hw);
 }
 
-static void *
+static cmp_func_f128_f128_t *
 __gekf2_resolve (void)
 {
-  return (void *) SW_OR_HW (__gekf2_sw, __gekf2_hw);
+  return SW_OR_HW (__gekf2_sw, __gekf2_hw);
 }
 
-static void *
+static cmp_func_f128_f128_t *
 __lekf2_resolve (void)
 {
-  return (void *) SW_OR_HW (__lekf2_sw, __lekf2_hw);
+  return SW_OR_HW (__lekf2_sw, __lekf2_hw);
 }
 
-static void *
+static cmp_func_f128_f128_t *
 __unordkf2_resolve (void)
 {
-  return (void *) SW_OR_HW (__unordkf2_sw, __unordkf2_hw);
+  return SW_OR_HW (__unordkf2_sw, __unordkf2_hw);
 }
 
 /* Resolve __nekf2, __gtkf2, __ltkf2 like __eqkf2, __gekf2, and __lekf2, since
    the functions return the same values.  */
 
-static void *
+static cmp_func_f128_f128_t *
 __nekf2_resolve (void)
 {
-  return (void *) SW_OR_HW (__eqkf2_sw, __eqkf2_hw);
+  return SW_OR_HW (__eqkf2_sw, __eqkf2_hw);
 }
 
-static void *
+static cmp_func_f128_f128_t *
 __gtkf2_resolve (void)
 {
-  return (void *) SW_OR_HW (__gekf2_sw, __gekf2_hw);
+  return SW_OR_HW (__gekf2_sw, __gekf2_hw);
 }
 
-static void *
+static cmp_func_f128_f128_t *
 __ltkf2_resolve (void)
 {
-  return (void *) SW_OR_HW (__lekf2_sw, __lekf2_hw);
+  return SW_OR_HW (__lekf2_sw, __lekf2_hw);
 }