Include math.h in nextafter-2.c test.

Include math.h in nextafter-2.c test.

I previously posted this with two other patches.  I've separated this into its
own patch.  What happens is because the nextafter-2.c test uses -fno-builtin,
and it does not include math.h, the wrong nextafterl and nextforwardl gets
called when long double is not IBM 128-bit (i.e. either 64-bit, or IEEE
128-bit).

Rather than add the include only for the PowerPC, I thought it was better to
always include it.  There might be some port in the future that has the same
issue with multiple long double types without using multilibs.

Can I check this into the master branch.

2020-11-15  Michael Meissner  <meissner@linux.ibm.com>

	* gcc.dg/nextafter-2.c: Include math.h.
---
 gcc/testsuite/gcc.dg/nextafter-2.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

On Sun, 2020-11-15 at 12:12 -0500, Michael Meissner via Gcc-patches wrote:
> Include math.h in nextafter-2.c test.
> 
> I previously posted this with two other patches.  I've separated this into its
> own patch.  What happens is because the nextafter-2.c test uses -fno-builtin,
> and it does not include math.h, the wrong nextafterl and nextforwardl gets
> called when long double is not IBM 128-bit (i.e. either 64-bit, or IEEE
> 128-bit).

Thats a sandbox issue, or something upstream ?

> 
> Rather than add the include only for the PowerPC, I thought it was better to
> always include it.  There might be some port in the future that has the same
> issue with multiple long double types without using multilibs.
> 
> Can I check this into the master branch.
> 
> 2020-11-15  Michael Meissner  <meissner@linux.ibm.com>
> 
> 	* gcc.dg/nextafter-2.c: Include math.h.
> ---
>  gcc/testsuite/gcc.dg/nextafter-2.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/gcc/testsuite/gcc.dg/nextafter-2.c b/gcc/testsuite/gcc.dg/nextafter-2.c
> index e51ae94be0c..8149a709fa5 100644
> --- a/gcc/testsuite/gcc.dg/nextafter-2.c
> +++ b/gcc/testsuite/gcc.dg/nextafter-2.c
> @@ -6,6 +6,18 @@
> 
>  #include <stdlib.h>
> 
> +/* In order to run on systems like the PowerPC that have 3 different long
> +   double types, include math.h so it can choose what is the appropriate
> +   nextafterl function to use.
> +
> +   If we didn't use -fno-builtin for this test, the PowerPC compiler would have
> +   changed the names of the built-in functions that use long double.  The
> +   nextafter-1.c function runs with this mapping.
> +
> +   Since this test uses -fno-builtin, include math.h, so that math.h can make
> +   the appropriate choice to use.  */



Can this be simplified to stl

/* Include math.h so that systems like PowerPC that have different long
double types can choose the appropriate nextafterl function to use.  */


> +#include <math.h>
> +
>  #if defined(__GLIBC__) && defined(__GLIBC_PREREQ)
>  # if !__GLIBC_PREREQ (2, 24)
>  /* Workaround buggy nextafterl in glibc 2.23 and earlier,
> -- 
> 2.22.0
> 
>

On Tue, Nov 17, 2020 at 11:33:23PM -0600, will schmidt wrote:
> On Sun, 2020-11-15 at 12:12 -0500, Michael Meissner via Gcc-patches wrote:
> > Include math.h in nextafter-2.c test.
> > 
> > I previously posted this with two other patches.  I've separated this into its
> > own patch.  What happens is because the nextafter-2.c test uses -fno-builtin,
> > and it does not include math.h, the wrong nextafterl and nextforwardl gets
> > called when long double is not IBM 128-bit (i.e. either 64-bit, or IEEE
> > 128-bit).
> 
> Thats a sandbox issue, or something upstream ?

I'm not sure what you are asking.  If you install the three critical IEEE
128-bit long double patches, and then configure a build with long double
defaulting to IEEE 128-bit, the nextafter-2 test will fail.

The reason is the nextafterl function in GLIBC assumes long double is IBM
128-bit extended double.  The __builtin_nextafterl function calls that
function.

If you compile it normally (with long double using IEEE 128-bit), the compiler
will automatically map nextafterl to __nextafterieee128.

Similarly if you include math.h, and use the -fno-builtin option, the math.h
library will still map nextafterl into __nextafterieee128, and the compiler
will call it.

However, if you do not include math.h and use the -fno-builtin option, the
compiler will call nextafterl, and get the wrong results, because the wrong
function was called.

What I meant in terms of the 3 patches being separated, the last time I posted
a patch for this problem, I grouped together 3 test suite failures into one
patch.  This time, I separated the cases into 3 separate patches (this one, the
fix for pr70117, and the fix for the decimal conversion test).

> > 
> > Rather than add the include only for the PowerPC, I thought it was better to
> > always include it.  There might be some port in the future that has the same
> > issue with multiple long double types without using multilibs.
> > 
> > Can I check this into the master branch.
> > 
> > 2020-11-15  Michael Meissner  <meissner@linux.ibm.com>
> > 
> > 	* gcc.dg/nextafter-2.c: Include math.h.
> > ---
> >  gcc/testsuite/gcc.dg/nextafter-2.c | 12 ++++++++++++
> >  1 file changed, 12 insertions(+)
> > 
> > diff --git a/gcc/testsuite/gcc.dg/nextafter-2.c b/gcc/testsuite/gcc.dg/nextafter-2.c
> > index e51ae94be0c..8149a709fa5 100644
> > --- a/gcc/testsuite/gcc.dg/nextafter-2.c
> > +++ b/gcc/testsuite/gcc.dg/nextafter-2.c
> > @@ -6,6 +6,18 @@
> > 
> >  #include <stdlib.h>
> > 
> > +/* In order to run on systems like the PowerPC that have 3 different long
> > +   double types, include math.h so it can choose what is the appropriate
> > +   nextafterl function to use.
> > +
> > +   If we didn't use -fno-builtin for this test, the PowerPC compiler would have
> > +   changed the names of the built-in functions that use long double.  The
> > +   nextafter-1.c function runs with this mapping.
> > +
> > +   Since this test uses -fno-builtin, include math.h, so that math.h can make
> > +   the appropriate choice to use.  */
> 
> 
> 
> Can this be simplified to stl
> 
> /* Include math.h so that systems like PowerPC that have different long
> double types can choose the appropriate nextafterl function to use.  */
> 
> 
> > +#include <math.h>
> > +
> >  #if defined(__GLIBC__) && defined(__GLIBC_PREREQ)
> >  # if !__GLIBC_PREREQ (2, 24)
> >  /* Workaround buggy nextafterl in glibc 2.23 and earlier,
> > -- 
> > 2.22.0
> > 
> > 

Sure, the comment is just trying to explain why math.h needs to be included.

On Wed, 2020-11-18 at 00:55 -0500, Michael Meissner wrote:
> On Tue, Nov 17, 2020 at 11:33:23PM -0600, will schmidt wrote:
> > On Sun, 2020-11-15 at 12:12 -0500, Michael Meissner via Gcc-patches 
> > wrote:
> > > Include math.h in nextafter-2.c test.
> > > 
> > > I previously posted this with two other patches.  I've separated
> > > this into its
> > > own patch.  What happens is because the nextafter-2.c test uses
> > > -fno-builtin,
> > > and it does not include math.h, the wrong nextafterl and
> > > nextforwardl gets
> > > called when long double is not IBM 128-bit (i.e. either 64-bit,
> > > or IEEE
> > > 128-bit).
> > 
> > Thats a sandbox issue, or something upstream ?
> 
> I'm not sure what you are asking.  If you install the three critical
> IEEE
> 128-bit long double patches, and then configure a build with long
> double
> defaulting to IEEE 128-bit, the nextafter-2 test will fail.

That answers my question.. this fixes an issue with patches that are
not upstream yet.  (your sandbox). 

> 
> The reason is the nextafterl function in GLIBC assumes long double is
> IBM
> 128-bit extended double.  The __builtin_nextafterl function calls
> that
> function.
> 
> If you compile it normally (with long double using IEEE 128-bit), the
> compiler
> will automatically map nextafterl to __nextafterieee128.
> 
> Similarly if you include math.h, and use the -fno-builtin option, the
> math.h
> library will still map nextafterl into __nextafterieee128, and the
> compiler
> will call it.
> 
> However, if you do not include math.h and use the -fno-builtin
> option, the
> compiler will call nextafterl, and get the wrong results, because the
> wrong
> function was called.
> 
> What I meant in terms of the 3 patches being separated, the last time
> I posted
> a patch for this problem, I grouped together 3 test suite failures
> into one
> patch.  This time, I separated the cases into 3 separate patches
> (this one, the
> fix for pr70117, and the fix for the decimal conversion test).
> 
> > > 
> > > Rather than add the include only for the PowerPC, I thought it
> > > was better to
> > > always include it.  There might be some port in the future that
> > > has the same
> > > issue with multiple long double types without using multilibs.
> > > 
> > > Can I check this into the master branch.
> > > 
> > > 2020-11-15  Michael Meissner  <meissner@linux.ibm.com>
> > > 
> > > 	* gcc.dg/nextafter-2.c: Include math.h.
> > > ---
> > >  gcc/testsuite/gcc.dg/nextafter-2.c | 12 ++++++++++++
> > >  1 file changed, 12 insertions(+)
> > > 
> > > diff --git a/gcc/testsuite/gcc.dg/nextafter-2.c
> > > b/gcc/testsuite/gcc.dg/nextafter-2.c
> > > index e51ae94be0c..8149a709fa5 100644
> > > --- a/gcc/testsuite/gcc.dg/nextafter-2.c
> > > +++ b/gcc/testsuite/gcc.dg/nextafter-2.c
> > > @@ -6,6 +6,18 @@
> > > 
> > >  #include <stdlib.h>
> > > 
> > > +/* In order to run on systems like the PowerPC that have 3
> > > different long
> > > +   double types, include math.h so it can choose what is the
> > > appropriate
> > > +   nextafterl function to use.
> > > +
> > > +   If we didn't use -fno-builtin for this test, the PowerPC
> > > compiler would have
> > > +   changed the names of the built-in functions that use long
> > > double.  The
> > > +   nextafter-1.c function runs with this mapping.
> > > +
> > > +   Since this test uses -fno-builtin, include math.h, so that
> > > math.h can make
> > > +   the appropriate choice to use.  */
> > 
> > 
> > 
> > Can this be simplified to stl
> > 
> > /* Include math.h so that systems like PowerPC that have different
> > long
> > double types can choose the appropriate nextafterl function to
> > use.  */
> > 
> > 
> > > +#include <math.h>
> > > +
> > >  #if defined(__GLIBC__) && defined(__GLIBC_PREREQ)
> > >  # if !__GLIBC_PREREQ (2, 24)
> > >  /* Workaround buggy nextafterl in glibc 2.23 and earlier,
> > > -- 
> > > 2.22.0
> > > 
> > > 
> 
> Sure, the comment is just trying to explain why math.h needs to be
> included.

Ok.   Your first paragraph in the comment clarifies that.  I'm
uncertain the rest of the comment helps, but i'll defer. 
Thanks. 

>

On Sun, Nov 15, 2020 at 12:12:34PM -0500, Michael Meissner wrote:
> --- a/gcc/testsuite/gcc.dg/nextafter-2.c
> +++ b/gcc/testsuite/gcc.dg/nextafter-2.c
> @@ -6,6 +6,18 @@
>  
>  #include <stdlib.h>
>  
> +/* In order to run on systems like the PowerPC that have 3 different long
> +   double types, include math.h so it can choose what is the appropriate
> +   nextafterl function to use.
> +
> +   If we didn't use -fno-builtin for this test, the PowerPC compiler would have
> +   changed the names of the built-in functions that use long double.  The
> +   nextafter-1.c function runs with this mapping.
> +
> +   Since this test uses -fno-builtin, include math.h, so that math.h can make
> +   the appropriate choice to use.  */
> +#include <math.h>

So if you use -fno-builtin (or just for some functions), and you don't
include <math.h>, things just break?  Nasty.

Of course such things aren't proper C (you *have to* include <math.h>
if you use functions from there), but how often will code like this
happen in practice :-/


The patch is okay for trunk.  Thanks!


Segher