[PATCH] PowerPC: PR libgcc/97543, build libgcc with -mno-gnu-attribute

[Alan Modra]

Hi Mike,
On Wed, Oct 28, 2020 at 08:42:04PM -0400, Michael Meissner via Gcc-patches wrote:
> PowerPC: PR libgcc/97543, fix 64-bit long double issues
> 
> There are two issues in PR libgcc/97543 which shows up if you build a GCC
> compiler with long double defaulting to 64-bit instead of 128-bit with IBM
> extended double:
> 
>     1)	The first issue was the t-linux file forced the entire libgcc library
> 	to be compiled with the -mlong-double-128 option.

Why is this is wrong?  If you are configuring using
--without-long-double-128 then that doesn't mean 128-bit long doubles
are unsupported, it just selects the default to be 64-bit long double.
A compiler built using --without-long-double-128 can generate code
for 128-bit long double by simply using -mlong-double-128.  In which
case you need the libgcc support for 128-bit long doubles.  Well, I
suppose you are passing -mlong-double-128 for those objects that need
it, but I can't see any harm in passing -mlong-double-128 everywhere
in libgcc.

It seems to me that *not* using -mlong-double-128 then opens you up to
the .gnu_attribute bug where we mark an object as using 64-bit long
double when it really is just using plain double.

> 
>     2)	The second issue is that the GNU attribute #4 is set to reflect using
> 	128-bit long doubles, and you get linker warnings when you use use the
> 	compiler, since libgcc_s.so indicates 128-bit IBM long doubles were
> 	used.  I ran into a similar issue with my patches to extend libgcc to
> 	work if long doubles were configured to use the 128-bit IEEE format
> 	instead of the 128-bit IBM format.
> 
> One feature of the current GNU attribute implementation is if you have a shared
> library (such as libgcc_s.so), the GNU attributes for the shared library is an
> inclusive OR of all of the modules within the library.

We do OR in non-conflicting attributes, but conflicting ones cause
errors, or are removed if the linker is given --no-warn-mismatch.  For
example:

cat > attr-ibm.s <<EOF
 .gnu_attribute 4,5
EOF
cat > attr-64.s <<EOF
 .gnu_attribute 4,9
EOF
powerpc64le-linux-as -o attr-ibm.o attr-ibm.s
powerpc64le-linux-readelf -A attr-ibm.o
powerpc64le-linux-as -o attr-64.o attr-64.s
powerpc64le-linux-readelf -A attr-64.o
powerpc64le-linux-ld -shared -o foo.so attr-ibm.o attr-64.o
#fails with an error
powerpc64le-linux-ld -shared --no-warn-mismatch -o foo.so attr-ibm.o attr-64.o
#succeeds
powerpc64le-linux-readelf -A foo.so
#displays nothing, conflicting attributes are removed

>  This means if any
> module uses the -mlong-double-128 option and uses long double, the GNU
> attributes for the library will indicate that it uses 128-bit IBM long
> doubles.  If you have a static library, you will get the warning only if you
> actually reference a module with the attribute set.
> 
> This patch does two things:
> 
>     1)	Instead of compiling the whole library with -mlong-double-128, it only
> 	compiles the modules that process the IBM extended double format with
> 	this switch.  It also specifies that these files must be compiled using
> 	the IBM format for long double.
> 
>     2)	I turned off GNU attributes for the whole library.  Originally, I just
> 	turned off GNU attributes for just the modules that process IBM
> 	extended format values.  But this doesn't work if the compiler defaults
> 	long double to 64-bits.  What happens is the logic in rs6000.c that
> 	sets the GNU attribute bits, will set the bits for 64-bit long double
> 	if a normal double (DFmode) is used.  So I just turned off the
> 	attributes for the whole library.
> 
> This patch replaces the patch I previously did for IEEE 128-bit to turn off
> GNU attributes for just the ibm-ldouble.o module.
> 	https://gcc.gnu.org/pipermail/gcc-patches/2020-October/556863.html
> 
> I have tested this by building a compiler on a little endian power9 system
> running Linux with long double defaulting to 64-bits using the configure
> option: --without-long-double-128, and I verified that the warning no longer is
> generated by the linker.
> 
> I then built a bootstrap compiler, by first building a non-bootstrap version.
> With that non-bootstrap compiler, I built versions of the MPC and MPFR.  Using
> those libraries, and the non-bootstrap compiler as the host compiler, I was
> able to do a full bootstrap compiler.
> 
> There are differences in the regression test suite where the test implicitly
> assumed long double was 128-bits or was a float128 test.
> 
> Can I install this patch into the master branch?  I would also like to install
> it in the GCC 10 branch after an appropriate period.
> 
> libgcc/
> 2020-10-28  Michael Meissner  <meissner@linux.ibm.com>
> 
> 	PR libgcc/97543
> 	* config/rs6000/t-linux (HOST_LIBGCC2_CFLAGS): Don't set
> 	-mlong-double-128 for all modules.  Instead set
> 	-mno-gnu-attributes.
> 	(IBM128_OBJS): New make variable for long double support.
> 	(IBM128_S_OBJS): New make variable for long double support.
> 	(IBM128_ALL_OBJS): New make variable for long double support.
> 	(IBM128_CFLAGS): New make variable for long double support.
> ---
>  libgcc/config/rs6000/t-linux | 18 +++++++++++++++++-
>  1 file changed, 17 insertions(+), 1 deletion(-)
> 
> diff --git a/libgcc/config/rs6000/t-linux b/libgcc/config/rs6000/t-linux
> index ed821947b66..b2a079c6b54 100644
> --- a/libgcc/config/rs6000/t-linux
> +++ b/libgcc/config/rs6000/t-linux
> @@ -1,6 +1,22 @@
>  SHLIB_MAPFILES += $(srcdir)/config/rs6000/libgcc-glibc.ver
>  
> -HOST_LIBGCC2_CFLAGS += -mlong-double-128
> +# On the modules that deal with IBM 128-bit values, we need to make sure that
> +# TFmode uses the IBM extended double format.
> +IBM128_OBJS	= ibm-ldouble$(objext) _powitf2$(objext) ppc64-fp$(objext) \
> +		  _divtc3$(object) _multc3$(object) \
> +		  _fixtfdi$(object) _fixunstfdi$(object) \
> +		  _floatditf$(objext) _floatunsditf$(objext)
> +
> +IBM128_S_OBJS	= $(patsubst %$(objext),%_s$(objext),$(IBM128_OBJS))
> +IBM128_ALL_OBJS	= $(IBM128_OBJS) $(IBM128_S_OBJS)
> +
> +IBM128_CFLAGS	= -mlong-double-128 -Wno-psabi -mabi=ibmlongdouble
> +
> +$(IBM128_ALL_OBJS) : INTERNAL_CFLAGS += $(IBM128_CFLAGS)
> +
> +# Turn off gnu attributes for the whole library.  This allows us to build
> +# libgcc that supports the different long double formats.
> +HOST_LIBGCC2_CFLAGS += -mno-gnu-attribute

It would be good if you could pass -mno-gnu-attribute just for the
libgcc_s.so objects.  That way you might save users from making
mistakes due to using the wrong libgcc.a somehow.  I think you may be
able to do that with

gcc_s_compile += -mno-gnu-attribute

And, yes, I completely agree that libgcc_s.so should not be marked
with .gnu_attribute.

>  # This is a way of selecting -mcmodel=small for ppc64, which gives
>  # smaller and faster libgcc code.  Directly specifying -mcmodel=small
> -- 
> 2.22.0
> 
> 
> -- 
> Michael Meissner, IBM
> IBM, M/S 2506R, 550 King Street, Littleton, MA 01460-6245, USA
> email: meissner@linux.ibm.com, phone: +1 (978) 899-4797

-- 
Alan Modra
Australia Development Lab, IBM