Re: [PATCH 2/2] Rework 128-bit complex multiply and divide.

["Kewen.Lin" <linkw@linux.ibm.com>]

Hi Mike,

on 2023/2/3 13:53, Michael Meissner wrote:
> This patch reworks how the complex multiply and divide built-in functions are
> done.  Previously we created built-in declarations for doing long double complex
> multiply and divide when long double is IEEE 128-bit.  The old code also did not
> support __ibm128 complex multiply and divide if long double is IEEE 128-bit.
> 
> This patch was originally posted on December 13th, 2022:
> 
> | Date: Tue, 13 Dec 2022 01:21:06 -0500
> | Subject: [PATCH V2] Rework 128-bit complex multiply and divide, PR target/107299
> | Message-ID: <Y5gZ0o1nzCq9MmR9@toto.the-meissners.org>
> 
> In terms of history, I wrote the original code just as I was starting to test
> GCC on systems where IEEE 128-bit long double was the default.  At the time, we
> had not yet started mangling the built-in function names as a way to bridge
> going from a system with 128-bit IBM long double to 128-bin IEEE long double.
> 
> The original code depends on there only being two 128-bit types invovled.  With
> the next patch in this series, this assumption will no longer be true.  When
> long double is IEEE 128-bit, there will be 2 IEEE 128-bit types (one for the
> explicit __float128/_Float128 type and one for long double).
> 
> The problem is we cannot create two separate built-in functions that resolve to
> the same name.  This is a requirement of add_builtin_function and the C front
> end.  That means for the 3 possible modes (IFmode, KFmode, and TFmode), you can
> only use 2 of them.
> 
> This code does not create the built-in declaration with the changed name.
> Instead, it uses the TARGET_MANGLE_DECL_ASSEMBLER_NAME hook to change the name
> before it is written out to the assembler file like it now does for all of the
> other long double built-in functions.
> 
> When I wrote these patches, I discovered that __ibm128 complex multiply and
> divide had originally not been supported if long double is IEEE 128-bit as it
> would generate calls to __mulic3 and __divic3.  I added tests in the testsuite
> to verify that the correct name (i.e. __multc3 and __divtc3) is used in this
> case.
> 
> I had previously sent this patch out on November 1st.  Compared to that version,
> this version no longer disables the special mapping when you are building
> libgcc, as it turns out we don't need it.
> 
> I tested all 3 patchs for PR target/107299 on:
> 
>     1)	LE Power10 using --with-cpu=power10 --with-long-double-format=ieee
>     2)	LE Power10 using --with-cpu=power10 --with-long-double-format=ibm
>     3)	LE Power9  using --with-cpu=power9  --with-long-double-format=ibm
>     4)	BE Power8  using --with-cpu=power8  --with-long-double-format=ibm
> 
> Once all 3 patches have been applied, we can once again build GCC when long
> double is IEEE 128-bit.  There were no other regressions with these patches.
> Can I check these patches into the trunk?

These two above paragraphs look a bit out of date (two patches now). :)

IIUC this patch actually fixes a latent issue, so it is independent of the one
fixing the bootstrapping issue, right?  This updated version of patch looks
good to me, but I'd leave the approval to Segher/David.  Thanks!

BR,
Kewen