Re: [PATCH] PowerPC: Map IEEE 128-bit long double built-in functions
On Fri, Dec 11, 2020 at 05:07:28PM -0500, Michael Meissner wrote:
> On Thu, Dec 10, 2020 at 03:20:01PM -0600, Segher Boessenkool wrote:
> > > We use the TARGET_MANGLE_DECL_ASSEMBLER_NAME hook to change this
> > > name. We
> > > - only do this if the default is that long double is IBM extended
> > > double, and
> > > - the user asked for IEEE 128-bit. */
> > > + only do this transformation if the __float128 type is enabled. This
> > > + prevents us from doing the transformation on older 32-bit ports that
> > > might
> > > + have enabled using IEEE 128-bit floating point as the default long
> > > double
> > > + type. */
> >
> > I still don't understand why you want to support some hypothetical and
> > untested configuration.
>
> I'm not sure what you mean by hypothetical and untested configuration.
Do you know any 32-bit config that uses IEEE QP float?
You should be able to use it (and on *all* systems even, that just
requires soft float for it, and it would make testing HUGELY easier, but
you prefer having ten times are much frustrating work later over having
a little bit of work now. But that aside.), but it isn't the default on
any 32-bit config. So it is hypothetical, and because of that, it
obviously cannot be tested.
> If you
> build a default configuration on a big endian system (i.e. do not use
> --with-cpu specifying at least power7), the __float128 type is not enabled by
> default, because you need access to the VSX vector registers.
It *cannot* be enabled on most older systems. This is a big problem.
If we *did* have soft float QP float, we could even enable it as the
default long double type on many systems. This is *much* less
problematic than switching the long double default on some complex
subset of configurations.
> > > + /* { dg-final { scan-assembler {\mbl __ynieee128} } } */
> >
> > This kind of thing does not portably work (the function names can have
> > various prefixes added).
>
> Why would it have prefixes? Can you suggest a better way of doing the test?
Many testcases do it already. Like, builtins-1.c has
{\mbl \.?_?__divdi3\M}. It depends on what ABI you have.
> There are lots of different long names that need to get mapped if long doubles
> are IEEE 128-bit. This test just tries to test every function to make sure it
> got mapped appropriately.
It isn't portable enough. I know you think that only powerpc64le-linux
has, and can ever have, and will ever have, QP float. But that is
wrong. And I don't want more work than necessary because you just
randomly restrict things.
You know this, I have said it tens of times already.
> We can't do a link test without requiring that GLIBC is 2.32 or newer, since
> the older GLIBCs don't have all of the symbols.
You should not require glibc for this at all. All compiler support
routines belong in libgcc. That is what libgcc *is*.
Not wanting to do that work gives crazy restrictions already, and will
be even more work in the future.
> And the link test would only
> work on little endian ELFV2 systems, since big endian GLIBC 2.32 does not
> export any of the float128 stuff.
Yes, more technical debt. (I hate that term, but it is appropriate
here).
> > I cannot understand this code, and it does seem far from obviously
> > correct. But, okay for trunk if you handle all fallout (and I mean all,
> > not just "all you consider important").
>
> Note, the code is only invokved for systems where IEEE 128-bit floating point
> is supported. So it will never get called for a lot of the random embedded
> systems or the big endian systems.
Did I say that?
I meant what I *said*, not whatever you imagine I meant.
Segher
Re: [PATCH] PowerPC: Map IEEE 128-bit long double built-in functions
On Thu, Dec 10, 2020 at 03:20:01PM -0600, Segher Boessenkool wrote:
> Hi!
>
> On Thu, Nov 19, 2020 at 06:58:14PM -0500, Michael Meissner wrote:
> > * config/rs6000/rs6000.c (rs6000_mangle_decl_assembler_name): Add
> > support for mapping built-in function names for long double
> > built-in functions if long double is IEEE 128-bit.
>
> Please write what it does, not "add support". Say what names it maps
> to, importantly. You don't need to list all, but what you wrote is
> 100% contentless.
Ok.
> > diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
> > index a5188553593..35e9c844e17 100644
> > --- a/gcc/config/rs6000/rs6000.c
> > +++ b/gcc/config/rs6000/rs6000.c
> > @@ -27065,57 +27065,128 @@ rs6000_globalize_decl_name (FILE * stream, tree
> > decl)
> > library before you can switch the real*16 type at compile time.
> >
> > We use the TARGET_MANGLE_DECL_ASSEMBLER_NAME hook to change this name.
> > We
> > - only do this if the default is that long double is IBM extended double,
> > and
> > - the user asked for IEEE 128-bit. */
> > + only do this transformation if the __float128 type is enabled. This
> > + prevents us from doing the transformation on older 32-bit ports that
> > might
> > + have enabled using IEEE 128-bit floating point as the default long
> > double
> > + type. */
>
> I still don't understand why you want to support some hypothetical and
> untested configuration.
I'm not sure what you mean by hypothetical and untested configuration. If you
build a default configuration on a big endian system (i.e. do not use
--with-cpu specifying at least power7), the __float128 type is not enabled by
default, because you need access to the VSX vector registers.
> > + /* { dg-final { scan-assembler {\mbl __ynieee128} } } */
>
> This kind of thing does not portably work (the function names can have
> various prefixes added).
Why would it have prefixes? Can you suggest a better way of doing the test?
There are lots of different long names that need to get mapped if long doubles
are IEEE 128-bit. This test just tries to test every function to make sure it
got mapped appropriately.
We can't do a link test without requiring that GLIBC is 2.32 or newer, since
the older GLIBCs don't have all of the symbols. And the link test would only
work on little endian ELFV2 systems, since big endian GLIBC 2.32 does not
export any of the float128 stuff.
> I cannot understand this code, and it does seem far from obviously
> correct. But, okay for trunk if you handle all fallout (and I mean all,
> not just "all you consider important").
Note, the code is only invokved for systems where IEEE 128-bit floating point
is supported. So it will never get called for a lot of the random embedded
systems or the big endian systems.
There is another way I could do the code that is simpler, but over time it will
need maintainence as new built-in functions are added.
Right now, in going over the system built-ins, it tries to do the mapping based
on the names:
1) If the function has a long double argument or returns long double, and
it ends in 'l', the name is mapped to "__" followed by
and then "ieee128". I.e. "sinl" becomes "__sinieee128".
2) However, some names do not follow that formula and I have a switch
statement for the outliers.
3) If the function name ends in "printf", the name is transformed to "__"
followed by followed by "ieee128". I.e. "sprintf" becomes
"__sprintfieee128".
4) If the function names ends in "scanf", the name is transformed to
"__isoc99_" followed by followed by "ieee128". I.e. "vscanf"
becomes "__isoc99_vscanfieee128".
I could change it to just having a switch statement of all known built-in
functions. I would need to add some code that if checking was enabled, it
would look for built-in functions that use long double that are not mapped, and
issue a warning to update the table of maps. This might be cleaner to look at,
but every so often, we would need to add a new symbol.
Would you prefer me to do that second implementation?
--
Michael Meissner, IBM
IBM, M/S 2506R, 550 King Street, Littleton, MA 01460-6245, USA
email: meiss...@linux.ibm.com, phone: +1 (978) 899-4797
Re: [PATCH] PowerPC: Map IEEE 128-bit long double built-in functions
Hi!
On Thu, Nov 19, 2020 at 06:58:14PM -0500, Michael Meissner wrote:
> * config/rs6000/rs6000.c (rs6000_mangle_decl_assembler_name): Add
> support for mapping built-in function names for long double
> built-in functions if long double is IEEE 128-bit.
Please write what it does, not "add support". Say what names it maps
to, importantly. You don't need to list all, but what you wrote is
100% contentless.
> diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
> index a5188553593..35e9c844e17 100644
> --- a/gcc/config/rs6000/rs6000.c
> +++ b/gcc/config/rs6000/rs6000.c
> @@ -27065,57 +27065,128 @@ rs6000_globalize_decl_name (FILE * stream, tree
> decl)
> library before you can switch the real*16 type at compile time.
>
> We use the TARGET_MANGLE_DECL_ASSEMBLER_NAME hook to change this name. We
> - only do this if the default is that long double is IBM extended double,
> and
> - the user asked for IEEE 128-bit. */
> + only do this transformation if the __float128 type is enabled. This
> + prevents us from doing the transformation on older 32-bit ports that might
> + have enabled using IEEE 128-bit floating point as the default long double
> + type. */
I still don't understand why you want to support some hypothetical and
untested configuration.
> + /* { dg-final { scan-assembler {\mbl __ynieee128} } } */
This kind of thing does not portably work (the function names can have
various prefixes added).
I cannot understand this code, and it does seem far from obviously
correct. But, okay for trunk if you handle all fallout (and I mean all,
not just "all you consider important").
Segher
Ping x2: [PATCH] PowerPC: Map IEEE 128-bit long double built-in functions
This patch is one of the critical patches to enable building GCC with the long double type set to IEEE 128-bit. I haven't received a response for this patch: | Date: Thu, 19 Nov 2020 18:58:14 -0500 | Subject: [PATCH] PowerPC: Map IEEE 128-bit long double built-in functions | Message-ID: <20201119235814.ga...@ibm-toto.the-meissners.org> | https://gcc.gnu.org/pipermail/gcc-patches/2020-November/559659.html -- Michael Meissner, IBM IBM, M/S 2506R, 550 King Street, Littleton, MA 01460-6245, USA email: meiss...@linux.ibm.com, phone: +1 (978) 899-4797
Ping [PATCH] PowerPC: Map IEEE 128-bit long double built-in functions
I haven't received a response for this patch: | Date: Thu, 19 Nov 2020 18:58:14 -0500 | Subject: [PATCH] PowerPC: Map IEEE 128-bit long double built-in functions | Message-ID: <20201119235814.ga...@ibm-toto.the-meissners.org> | https://gcc.gnu.org/pipermail/gcc-patches/2020-November/559659.html -- Michael Meissner, IBM IBM, M/S 2506R, 550 King Street, Littleton, MA 01460-6245, USA email: meiss...@linux.ibm.com, phone: +1 (978) 899-4797
[PATCH] PowerPC: Map IEEE 128-bit long double built-in functions
[PATCH] PowerPC: Map IEEE 128-bit long double built-in functions.
I posted this patch by accident to an internal IBM mailing list instead of
gcc-patches.
This patch replaces patches previously submitted:
September 24th, 2020:
Message-ID: <20200924203159.ga31...@ibm-toto.the-meissners.org>
October 9th, 2020:
Message-ID: <20201009043543.ga11...@ibm-toto.the-meissners.org>
October 24th, 2020:
Message-ID: <2020100346.ga8...@ibm-toto.the-meissners.org>
This patch maps the built-in functions that take or return long double
arguments on systems where long double is IEEE 128-bit.
This patch goes through the built-in functions and changes the name of the
math, scanf, and printf built-in functions to use the functions that GLIBC
provides when long double uses the IEEE 128-bit representation.
In addition, changing the name in GCC allows the Fortran compiler to
automatically use the correct name.
To map the math functions, typically this patch changes l to
__ieee128. However there are some exceptions that are handled with this
patch.
To map the printf functions, is mapped to __ieee128.
To map the scanf functions, is mapped to __isoc99_ieee128.
With the other IEEE long double patches, I have tested this patch by building 3
bootstrap compilers on a little endian power9 system, using the Advance
Toolchain AT14.0 library, which uses GLIBC 2.32:
1) One compiler defaulted long double to IBM extended double;
2) One compiler defaulted long double to IEEE 128-bit; (and)
3) One compiler defaulted long double to 64 bit.
I was able to bootstrap each compiler and run make check. In addition for the
compilers using the two 128-bit long double types (IBM, IEEE), I have built the
spec 2017 benchmark for both power9 and power10.
At the moment, there are some differences between between the three runs for
make check. I have some patches to fix these issue that I've done in the past,
and I will be working on resubmitting them in the future.
In addition, there are 3 fortran benchmarks (ieee/large_2.f90,
default_format_2.f90, and default_format_denormal_2.f90) that now pass when the
long double default is IEEE 128-bit.
Can I check this into the master branch?
gcc/
2020-11-17 Michael Meissner
* config/rs6000/rs6000.c (rs6000_mangle_decl_assembler_name): Add
support for mapping built-in function names for long double
built-in functions if long double is IEEE 128-bit.
gcc/testsuite/
2020-11-17 Michael Meissner
* gcc.target/powerpc/float128-longdouble-math.c: New test.
* gcc.target/powerpc/float128-longdouble-stdio.c: New test.
* gcc.target/powerpc/float128-math.c: Adjust test for new name
being generated. Add support for running test on power10. Add
support for running if long double defaults to 64-bits.
---
gcc/config/rs6000/rs6000.c| 135 --
.../powerpc/float128-longdouble-math.c| 442 ++
.../powerpc/float128-longdouble-stdio.c | 36 ++
.../gcc.target/powerpc/float128-math.c| 16 +-
4 files changed, 589 insertions(+), 40 deletions(-)
create mode 100644 gcc/testsuite/gcc.target/powerpc/float128-longdouble-math.c
create mode 100644 gcc/testsuite/gcc.target/powerpc/float128-longdouble-stdio.c
diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
index a5188553593..35e9c844e17 100644
--- a/gcc/config/rs6000/rs6000.c
+++ b/gcc/config/rs6000/rs6000.c
@@ -27065,57 +27065,128 @@ rs6000_globalize_decl_name (FILE * stream, tree decl)
library before you can switch the real*16 type at compile time.
We use the TARGET_MANGLE_DECL_ASSEMBLER_NAME hook to change this name. We
- only do this if the default is that long double is IBM extended double, and
- the user asked for IEEE 128-bit. */
+ only do this transformation if the __float128 type is enabled. This
+ prevents us from doing the transformation on older 32-bit ports that might
+ have enabled using IEEE 128-bit floating point as the default long double
+ type. */
static tree
rs6000_mangle_decl_assembler_name (tree decl, tree id)
{
- if (!TARGET_IEEEQUAD_DEFAULT && TARGET_IEEEQUAD && TARGET_LONG_DOUBLE_128
+ if (TARGET_FLOAT128_TYPE && TARGET_IEEEQUAD && TARGET_LONG_DOUBLE_128
&& TREE_CODE (decl) == FUNCTION_DECL
- && DECL_IS_UNDECLARED_BUILTIN (decl))
+ && DECL_IS_UNDECLARED_BUILTIN (decl)
+ && DECL_BUILT_IN_CLASS (decl) == BUILT_IN_NORMAL)
{
size_t len = IDENTIFIER_LENGTH (id);
const char *name = IDENTIFIER_POINTER (id);
+ char *newname = NULL;
- if (name[len - 1] == 'l')
+ /* See if it is one of the built-in functions with an unusual name. */
+ switch (DECL_FUNCTION_CODE (decl))
{
- bool uses_ieee128_p = false;
- tree type = TREE_TYPE (decl);
- machine_mode ret_mode = TYPE_MODE (ty
