public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Michael Meissner <meissner@linux.ibm.com>
To: gcc-patches@gcc.gnu.org,
	Michael Meissner <meissner@linux.ibm.com>,
	Segher Boessenkool <segher@kernel.crashing.org>,
	"Kewen.Lin" <linkw@linux.ibm.com>,
	David Edelsohn <dje.gcc@gmail.com>,
	Peter Bergner <bergner@linux.ibm.com>,
	Will Schmidt <will_schmidt@vnet.ibm.com>
Subject: [PATCH, V3] PR 107299, GCC does not build on PowerPC when long double is IEEE 128-bit
Date: Wed, 14 Dec 2022 15:29:02 -0500	[thread overview]
Message-ID: <Y5oyDgX3SgN5W8Pl@toto.the-meissners.org> (raw)

This set of patches was first submitted on November 1st.  Kewen.Lin
<linkw@linux.ibm.com> asked for some changes to the first set of patches.  I
also tried to clean up the comments in the second patch about types that Segher
Boessenkool <segher@kernel.crashing.org> mentioned.

I had just re-submitted the first patch yesterday, but Segher asked that I
repost all three patches.  Here is the original commentary for all three
patches, tweaked a little bit:

These 3 patches fix the problems with building GCC on PowerPC systems when long
double is configured to use the IEEE 128-bit format.

There are 3 patches in this patch set.  The first two patches are required to
fix the basic problem.  The third patch fixes some issues that were noticed
along the way.

The basic issue is internally within GCC there are several types for 128-bit
floating point.  The types are:

    1) The long double type (which use either TFmode for 128-bit long doubles
        or possibly DFmode for 64-bit long doubles).  In the normal case, long
        double is 128-bits (TFmode) and depending on the configuration options
        and the switches passed by the user at compilation time, long double is
        either the 128-bit IBM double-double type or IEEE 128-bit.

    2)  The type for __ibm128.  If long double is IBM 128-bit double-double,
        internally within the compiler, this type is the same as the long
        double type.  If long double is either IEEE 128-bit or is 64-bit, then
        this type is a separate type.  If long double is not double-double,
        this type will use IFmode during RTL.

    3)  The type for _Float128.  This type is always IEEE 128-bit if it exists.
        While it is a separate internal type, currently if long double is IEEE
        128-bit, this type uses TFmode once it gets to RTL, but within Gimple
        it is a separate type.  If long double is not IEEE 128-bit, then this
        type uses KFmode.  All of the f128 math functions defined by the
        compiler use this type.  In the past, _Float128 was a C extended type
        and not available in C++.  Now it is a part of the C/C++ 2x standards.

    4)  The type for __float128.  The history is I implemented __float128
        first, and several releases later, we added _Float128 as a standard C
        type.  Unfortunately, I didn't think things through enough when
        _Float128 came out.  Like __ibm128, it uses the long double type if
        long double is IEEE 128-bit, and now it uses the _Float128 type if long
        double is not IEEE 128-bit.  IMHO, this is the major problem.  The two
        IEEE 128-bit types should use the same type internally (or at least one
        should be a qualified type of the other).  Before we started adding
        more support for _Float128, it mostly works, but now it doesn't with
        more optimizations being done.

    5)  The error occurs in building _mulkc3 in libgcc, when the TFmode type in
        the code is defined to use attribute((mode(TF))), but the functions
        that are called all have _Float128 arguments.  These are separate
        types, and ultimately one of the consistancy checks fails because they
        are different types.

There are 3 patches in this set:

    1)  The first patch rewrites how the complex 128-bit multiply and divide
        functions are done in the compiler.  In the old scheme, essentially
        there were only two types ever being used, the long double type, and
        the not long double type.  The original code would make the names
        called of these functions to be __multc3/__divtc3 or
        __mulkc3/__divkc3.  This worked because there were only two types.
        With straightening out the types, so __float128/_Float128 is never the
        long double type, there are potentially 3-4 types.  However, the C
        front end and the middle end code will not let us create two built-in
        functions that have the same name.

        Patch #1 patch rips out this code, and rewrites it to be cleaner.

	In the original version of the patches, I disabled doing the mapping
	when building libgcc because it caused problems when building __mulkc3
	and __divkc3.  I have removed this check, since the second patch will
	allow these functions to be built without disabling the mapping.

    2)  The second patch fixes the problem of __float128 and _Float128 not
        being the same if long double is IEEE 128-bit.  After this patch, both
        _Float128 and __float128 types will always use the same type.  When we
        get to RTL, it will always use KFmode type (and not use TFmode).  The
        stdc++ library will not build if we use TFmode for these types due to
        the other changes.

        There is a minor codegen issue that if you explicitly use long double
        and call the F128 FMA (fused multiply-add) round to odd functions that
        are defined to use __float128/_Float128 arguments.  While we might be
        able to optimize these later, I don't think it is important to optimize
        the use of long double instead of __float128/_Float128.  Note, if you
        use the proper __float128/_Float128 types instead of long double, the
        code does the optimization.

	The issue is the extra conversions that are inserted to convert between
	KFmode and TFmode won't allow the combiner to use the current patterns
	to combine the multiply, add/subtract, and optionally negation into the
	single FMA instruction.

        By doing this change, it also fixes two tests that have been broken on
        IEEE 128-bit long double systems (float128-cmp2-runnable.c and
        nan128-1.c).  These two tests use __float128 variables and call nansq
        to create a signaling NaN.  Nansq is defined to be __builtin_nansf128,
        which returns a _Float128 Nan.  However, since in the current
        implementation before these patches, __float128 is a different type
        than _Float128 when long double is IEEE 128-bit, the machine
        independent code converts the signaling NaN into a non-signaling NaN.
        Since after these patches they are the same internal type, the
        signaling NaN is preserved.

    3)  Patch #3 fixes two tests that were still failing after patches #1 and
        #2 were applied (convert-fp-128.c and pr85657-3.c).  This patch fixes
	the conversions between the 128-bit floating point types.  In the past,
        we would always call rs6000_expand_float128_convert to do all
        conversions.  After this patch, the conversions between different types
        that have the same representation will be done in-line and not call the
        rs6000_expand_float128_convert function.

        In addition, in the past we missed some conversions, and the compiler
        would generate an external call, even though the types might have the
        same representation.

After these patches, there are 3 specific tests and 1 set of tests that fail
when using IEEE 128-bit long double:

    1)  fp128_conversions.c: I haven't looked at yet;

    2)  pr105334.c: This is a bug that __ibm128 doesn't work if the default
        long double is IEEE 128-bit and you use the options: -mlong-double-128
        -msoft-float (i.e. no -mabi=ibmlongdouble).  I believe I have patches
        for this floating around.

    3)  The g++.dg/cpp23/ext-floating1.C test is failing.  I believe we need to
        dig in to fix PowerPC specific ISO C/C++ 2x _Float128 support.  I have
        looked at it yet.

    4)  All/some of the G++ modules tests fail.  This is PR 98645, and it is
        assigned to Nathan Sidwell.

-- 
Michael Meissner, IBM
PO Box 98, Ayer, Massachusetts, USA, 01432
email: meissner@linux.ibm.com

             reply	other threads:[~2022-12-14 20:29 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-12-14 20:29 Michael Meissner [this message]
2022-12-14 20:32 ` [PATCH 1/3, V3] PR 107299, Rework 128-bit complex multiply and divide Michael Meissner
2022-12-14 20:34 ` [PATCH 2/3, V3] PR 107299, Make __float128 use the _Float128 type Michael Meissner
2022-12-14 20:35 ` [PATCH 3/3, V3] PR 107299, Update float 128-bit conversion Michael Meissner
2023-02-27 23:26 ` Ping: [PATCH, V3] PR 107299, GCC does not build on PowerPC when long double is IEEE 128-bit Michael Meissner
2023-03-02 20:26 ` Segher Boessenkool

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=Y5oyDgX3SgN5W8Pl@toto.the-meissners.org \
    --to=meissner@linux.ibm.com \
    --cc=bergner@linux.ibm.com \
    --cc=dje.gcc@gmail.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=linkw@linux.ibm.com \
    --cc=segher@kernel.crashing.org \
    --cc=will_schmidt@vnet.ibm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).