Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <9d11aef7-b8c5-7f88-ab2c-e39aae79f01e@sholland.org>
Date: Mon, 1 Jul 2019 19:48:36 -0500
From: Samuel Holland <samuel@...lland.org>
To: musl@...ts.openwall.com
Subject: Re: [GCC PATCH] powerpc64 musl libc support for IEEE binary128
 long double

On 7/1/19 12:42 PM, Rich Felker wrote:
> On Sun, Jun 30, 2019 at 02:38:25PM -0500, Samuel Holland wrote:
>> This works properly with multiarch and -m64/-m32:
> 
> musl doesn't support multilib 

I know that.

> or gcc style multiarch

[citation needed]

> (sharing same include path for multiple musl archs),

"gcc style multiarch" adds the multiarch triple as a suffix to the include path,
and only uses the (shared) unsuffixed path as a fallback:

$ gcc -m64 -v -xc /dev/null |& grep /usr/include
 /usr/include/powerpc64-linux-musl
 /usr/include
$ gcc -m32 -v -xc /dev/null |& grep /usr/include
 /usr/include/powerpc-linux-musl
 /usr/include

Unfortunately, software that installs headers into ${includedir}/${pkgname}
tends to have reverse dependencies that hardcode /usr/include/${pkgname} in
their configuration scripts. So it's much easier to leave includedir as
/usr/include and only move the arch-specific headers to the arch-specific header
directory (which my current distro has machinery to do already):

$ gcc -E -xc - <<< "#include <stdio.h>" | grep include


# 1 "/usr/include/stdc-predef.h" 1 3 4
# 1 "/usr/include/stdio.h" 1 3 4
# 1 "/usr/include/features.h" 1 3 4
# 9 "/usr/include/stdio.h" 2 3 4
# 26 "/usr/include/stdio.h" 3 4
# 1 "/usr/include/powerpc64-linux-musl/bits/alltypes.h" 1 3 4
# 6 "/usr/include/powerpc64-linux-musl/bits/alltypes.h" 3 4
# 88 "/usr/include/powerpc64-linux-musl/bits/alltypes.h" 3 4
# 103 "/usr/include/powerpc64-linux-musl/bits/alltypes.h" 3 4
# 190 "/usr/include/powerpc64-linux-musl/bits/alltypes.h" 3 4
# 348 "/usr/include/powerpc64-linux-musl/bits/alltypes.h" 3 4
# 27 "/usr/include/stdio.h" 2 3 4
# 54 "/usr/include/stdio.h" 3 4

So no, multiarch is not multilib. And yes, multiarch does exactly what you wish
multilib did. And yes, I've explained this before. Several times!

> so that's not a constraint anyway.

The "musl doesn't support multilib" trope is unhelpful when it's used as an
excuse to hand-wave away all forms of compiler multitargeting, especially ones
that currently work.

>> diff --git a/gcc/config/rs6000/linux.h b/gcc/config/rs6000/linux.h
>> index 96b97877989b..439b5179b172 100644
>> --- a/gcc/config/rs6000/linux.h
>> +++ b/gcc/config/rs6000/linux.h
>> @@ -139,8 +139,9 @@
>>  #define POWERPC_LINUX
>>  
>>  /* ppc linux has 128-bit long double support in glibc 2.4 and later.  */
>> +/* musl supports 128-bit long double in 1.1.23 and later on powerpc64 only.  */
>>  #ifdef TARGET_DEFAULT_LONG_DOUBLE_128
>> -#define RS6000_DEFAULT_LONG_DOUBLE_SIZE 128
>> +#define RS6000_DEFAULT_LONG_DOUBLE_SIZE (OPTION_MUSL ? 64 : 128)
>>  #endif
> 
> This looks uncontroversial, but since this is only for the 32-bit case
> I don't think it needs comments about musl ppc64.

Okay, that makes sense; I'll remove the comment here.

> Also, as I'm trying to roll the release in the next few days and this
> is a somewhat questionable change I don't yet understand, it won't be
> in musl 1.1.23.

Sure, and I haven't sent this patch to GCC yet either. When that happens, I'll
fill in whatever the version ends up being. I was trying to be consistent with
the glibc comment, and it seemed helpful information to have.

>>  /* Static stack checking is supported by means of probes.  */
>> diff --git a/gcc/config/rs6000/linux64.h b/gcc/config/rs6000/linux64.h
>> index 5380f6a6a6f1..2b76255f7673 100644
>> --- a/gcc/config/rs6000/linux64.h
>> +++ b/gcc/config/rs6000/linux64.h
>> @@ -447,12 +447,18 @@ extern int dot_symbols;
>>  ":%(dynamic_linker_prefix)/lib64/ld64.so.1}"
>>  #endif
>>  
>> +#ifdef TARGET_DEFAULT_LONG_DOUBLE_128
>> +#define MUSL_DYNAMIC_LINKER_FP "%{mlong-double-64:;:-ieee128}"
>> +#else
>> +#define MUSL_DYNAMIC_LINKER_FP "%{mlong-double-128:-ieee128}"
>> +#endif
> 
> I'm not a fan of the "ieee128" naming, simply because "ieee128"
> presumably is the name of some unrelated ieee document and doesn't
> suggest it's got anything to do with floating point except to someone
> who assumes ieee means ieee754. My first thought would have been
> "quad" or "qf" or even "ieeequad". But this is a relatively minor
> issue.

And I like my bikeshed blue. As I mentioned in the other patch:
> So far the suggested ABI names have been "ld128", "ieee128", "ieeequad",
> and "f128".

Can we please pick something so I can stop recompiling my entire system :)

>>  /* ppc{32,64} linux has 128-bit long double support in glibc 2.4 and later.  */
>> +/* musl supports 128-bit long double in 1.1.23 and later on powerpc64 only.  */
>>  #ifdef TARGET_DEFAULT_LONG_DOUBLE_128
>> -#define RS6000_DEFAULT_LONG_DOUBLE_SIZE 128
>> +#define RS6000_DEFAULT_LONG_DOUBLE_SIZE (OPTION_MUSL && !TARGET_64BIT ? 64 : 128)
>>  #endif
> 
> Default ABI for musl should not be changed. I guess this default
> wasn't being used anyway and was set by configure's logic for musl (or
> for !glibc?) but it was decided very intentionally when musl's ppc64
> port was added that the ABI would be ld64 because that was the only
> thing that non-bleeding-edge tooling could support, and we didn't want
> to mandate gcc8+ or whatever version would have been needed (maybe 7+?
> not sure) to build ppc64.

This isn't changing the *default* default long double size. This is only
modifying the runtime default in the case gcc was configured with
--with-long-double-128. In rs6000.c theres:

#ifndef RS6000_DEFAULT_LONG_DOUBLE_SIZE
#define RS6000_DEFAULT_LONG_DOUBLE_SIZE 64
#endif

The ultimate default (with no ./configure option) is still 64 bits.

> Beyond that, before we move further with this I want to understand the
> motivation for it. If it's just that clang doesn't presently support
> ld64 (not clear if that's true but it looked like it might be from
> some comments I saw on #musl), this is not going upstream in musl
> unless/until clang supports ld64. What I absolutely *don't* want is to
> make it so there are two separate ABIs for musl-ppc64 that are "the
> standard/gcc ABI" and "the clang/gcc8+ ABI". That's just creating
> tooling fragmentation hell.

Clang requires patching in either case, since it's hardcoded to 128-bit IBM
double-double[1] and not configurable. This patch has nothing at all to do with
clang. And nowhere have I suggested that the default ABI would change.

[1]:
https://github.com/llvm/llvm-project/blob/745379a0af74a37465f616b99c10a09b4f0d2add/clang/lib/Basic/Targets/PPC.h#L78

> If the motivation is that there are musl-ppc64 users who really want
> quad math, and want to use it via long double rather than _Float128
> for whatever reason, then the idea behind adding a second ABI here is
> at least reasonable. I wish it could be supported in old gcc too, but
> I'm told the patch to add quad support was very invasive and hard to
> backport, so I'm guessing that's not happening.

The support was added in GCC 7 (which is already two years old). Considering
that the GCC 6 branch is closed, I highly doubt anything will be backported.

> Rich

Samuel



Powered by blists - more mailing lists

Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.