Bug 1934065 - clang++ 12 not compatible with <vector> on ppc64le
Summary: clang++ 12 not compatible with <vector> on ppc64le
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: clang
Version: rawhide
Hardware: ppc64le
OS: Unspecified
unspecified
unspecified
Target Milestone: ---
Assignee: Tom Stellard
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
: 1933252 (view as bug list)
Depends On:
Blocks: PPCTracker 1933742
TreeView+ depends on / blocked
 
Reported: 2021-03-02 12:30 UTC by Dan Horák
Modified: 2021-04-13 04:47 UTC (History)
10 users (show)

Fixed In Version: clang-12.0.0-0.7.rc3.fc35 clang-12.0.0-0.7.rc3.eln110 clang-12.0.0-0.3.rc1.fc34
Doc Type: If docs needed, set a value
Doc Text:
Clone Of:
Environment:
Last Closed: 2021-04-05 00:17:41 UTC
Type: Bug
Embargoed:


Attachments (Terms of Use)

Description Dan Horák 2021-03-02 12:30:11 UTC
Description of problem:
When the <vector> C++ header is included, then clang++ fails with

<mock-chroot> sh-5.1# clang++ -o t.o t.cpp
In file included from t.cpp:1:
In file included from /usr/lib/gcc/ppc64le-redhat-linux/11/../../../../include/c++/11/vector:60:
In file included from /usr/lib/gcc/ppc64le-redhat-linux/11/../../../../include/c++/11/bits/stl_algobase.h:63:
/usr/lib/gcc/ppc64le-redhat-linux/11/../../../../include/c++/11/ext/numeric_traits.h:222:38: error: use of undeclared identifier '__ieee128'
    struct __numeric_traits_floating<__ieee128>
                                     ^
/usr/lib/gcc/ppc64le-redhat-linux/11/../../../../include/c++/11/ext/numeric_traits.h:230:29: error: use of undeclared identifier '__ieee128'
    struct __numeric_traits<__ieee128>
                            ^
/usr/lib/gcc/ppc64le-redhat-linux/11/../../../../include/c++/11/ext/numeric_traits.h:231:40: error: use of undeclared identifier '__ieee128'
    : public __numeric_traits_floating<__ieee128>
                                       ^
3 errors generated.

The test code is

<mock-chroot> sh-5.1# cat t.cpp
#include <vector>

int main(void)
{
    return 0;
}


There wasn't such problem when clang/llvm 11 was in the buildroots. I suspect some compatibility issue between gcc 11 and clang/llvm 12.


Version-Release number of selected component (if applicable):
clang-12.0.0-0.1.rc1.fc35.ppc64le

Comment 1 Dan Horák 2021-03-02 12:37:38 UTC
and same result from clang-12.0.0-0.3.rc2.fc35.ppc64le

Comment 2 Dan Horák 2021-03-02 12:48:53 UTC
and things work OK with clang-11.1.0-0.4.rc2.fc34.ppc64le

Comment 3 Jonathan Wakely 2021-03-02 17:12:23 UTC
I think this should be fixed in libstdc++:

--- a/libstdc++-v3/include/ext/numeric_traits.h
+++ b/libstdc++-v3/include/ext/numeric_traits.h
@@ -219,7 +219,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 # elif defined __LONG_DOUBLE_IBM128__
   // long double is __ibm128, define traits for __ieee128
   template<>
-    struct __numeric_traits_floating<__ieee128>
+    struct __numeric_traits_floating<__float128>
     {
       static const int __max_digits10 = 36;
       static const bool __is_signed = true;
@@ -227,8 +227,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
       static const int __max_exponent10 = 4932;
     };
   template<>
-    struct __numeric_traits<__ieee128>
-    : public __numeric_traits_floating<__ieee128>
+    struct __numeric_traits<__float128>
+    : public __numeric_traits_floating<__float128>
     { };
 # endif
 #endif

Comment 4 serge_sans_paille 2021-03-02 19:49:53 UTC
I confirm `__float128` is supported by clang.

Comment 5 Dan Horák 2021-03-04 16:28:00 UTC
Jon, Serge, what should be the next step? Switching to gcc/libstdc++? It's currently blocking Firefox builds in Rawhide and soon in F-34.

Comment 6 serge_sans_paille 2021-03-13 07:54:44 UTC
@Dan: I have a patch pending upstream https://reviews.llvm.org/D97846 once it's accepted I'll backport it to Fedora.

Comment 7 serge_sans_paille 2021-03-17 20:32:02 UTC
Should be fixed by clang-12.0.0-0.7.rc3.fc35

Comment 8 Dan Horák 2021-03-17 21:54:59 UTC
Serge, we need a build for F-34 too, it contains clang 12 as well.

Comment 9 Fedora Update System 2021-03-17 23:42:39 UTC
FEDORA-2021-616c8ade32 has been pushed to the Fedora 35 stable repository.
If problem still persists, please make note of it in this bug report.

Comment 10 Fedora Update System 2021-03-18 03:45:44 UTC
FEDORA-2021-09931afa52 has been pushed to the Fedora ELN stable repository.
If problem still persists, please make note of it in this bug report.

Comment 11 Fedora Update System 2021-03-19 06:13:15 UTC
FEDORA-2021-0c26339f28 has been submitted as an update to Fedora 34. https://bodhi.fedoraproject.org/updates/FEDORA-2021-0c26339f28

Comment 12 Fedora Update System 2021-03-19 18:45:43 UTC
FEDORA-2021-0c26339f28 has been pushed to the Fedora 34 testing repository.
Soon you'll be able to install the update with the following command:
`sudo dnf upgrade --enablerepo=updates-testing --advisory=FEDORA-2021-0c26339f28`
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2021-0c26339f28

See also https://fedoraproject.org/wiki/QA:Updates_Testing for more information on how to test updates.

Comment 13 Jakub Jelinek 2021-03-20 13:36:48 UTC
In https://koji.fedoraproject.org/koji/taskinfo?taskID=64172884
I'm getting a different error on the same thing:
clang++ -D_GNU_SOURCE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS  -shared -fPIC -I./.. -lLLVM annobin.cpp -o annobin-for-clang.so
In file included from annobin.cpp:15:
In file included from /usr/include/clang/Frontend/FrontendPluginRegistry.h:16:
In file included from /usr/include/clang/Frontend/FrontendAction.h:21:
In file included from /usr/include/clang/Basic/LLVM.h:21:
In file included from /usr/include/llvm/Support/Casting.h:20:
In file included from /usr/lib/gcc/ppc64le-redhat-linux/11/../../../../include/c++/11/memory:63:
In file included from /usr/lib/gcc/ppc64le-redhat-linux/11/../../../../include/c++/11/bits/stl_algobase.h:63:
/usr/lib/gcc/ppc64le-redhat-linux/11/../../../../include/c++/11/ext/numeric_traits.h:222:38: error: __float128 is not supported on this target
    struct __numeric_traits_floating<__ieee128>
                                     ^
/usr/lib/gcc/ppc64le-redhat-linux/11/../../../../include/c++/11/ext/numeric_traits.h:230:29: error: __float128 is not supported on this target
    struct __numeric_traits<__ieee128>
                            ^
/usr/lib/gcc/ppc64le-redhat-linux/11/../../../../include/c++/11/ext/numeric_traits.h:231:40: error: __float128 is not supported on this target
    : public __numeric_traits_floating<__ieee128>
                                       ^

Comment 14 serge_sans_paille 2021-03-20 15:22:10 UTC
Jakub, I think you need -mfloat128 (?)

Comment 15 Jakub Jelinek 2021-03-20 15:31:52 UTC
Well, if one can't include <memory> or other STL headers without that option on ppc64le, it is bad.
Dunno, perhaps libstdc++ needs to have workaround for this and only do those if __SIZEOF_FLOAT128__ or whatever other macro that says whether __float128 or __ieee128 can be used is defined.
In GCC  __float128 on powerpc64le is available always, not just based on compiler options, one can change what long double is (double vs. __ibm128 vs. __float128/__ieee128) through command line options.

Comment 16 Jonathan Wakely 2021-03-21 12:41:50 UTC
(In reply to Jakub Jelinek from comment #15)
> Dunno, perhaps libstdc++ needs to have workaround for this and only do those
> if __SIZEOF_FLOAT128__ or whatever other macro that says whether __float128
> or __ieee128 can be used is defined.

We already have such macros, they're just not working correctly for clang (apparently).

Comment 17 Jonathan Wakely 2021-03-21 12:51:34 UTC
# elif defined __LONG_DOUBLE_IBM128__
  // long double is __ibm128, define traits for __ieee128
  template<>
    struct __numeric_traits_floating<__ieee128>

We only define that specialization if __LONG_DOUBLE_IBM128__ is defined, which means that long double is double double (i.e. IBM 128-bit float). 

(In reply to Jakub Jelinek from comment #15)
> In GCC  __float128 on powerpc64le is available always, not just based on
> compiler options, one can change what long double is (double vs. __ibm128
> vs. __float128/__ieee128) through command line options.

Right. It looks like clang supports -mabi=ibmlongdouble without __float128 (and it's the default, which is a very weird choice). That's an additional ABI that GCC doesn't support and so I am going to go and slam my head in a door until I no longer have to think about this topic.


We could change the #elif above to also check that __SIZEOF_FLOAT128__ is present, but I think it would be better to make this false when __SIZEOF_FLOAT128__ isn't defined:

#ifdef _GLIBCXX_LONG_DOUBLE_ALT128_COMPAT

If the compiler only supports one type of 128-bit float (the double-double one) then we shouldn't be enabling the "ALT128_COMPAT" code. That code is specifically intended to support the case where there are two different 128-bit floating-point types.

That means the -mabi=ibmlongdouble -mno-float128 ABI won't be supported in libstdc++ with Clang. I cannot find the strength to add yet another variation to this code. I don't care if it's clang's default, it's still stupid.

Comment 18 Jakub Jelinek 2021-03-21 12:54:01 UTC
Well, all I see it using is currently _GLIBCXX_LONG_DOUBLE_ALT128_COMPAT which is configure time decision (so based on what gcc supports, not necessarily the compiler used to compile the header), and
then for __LONG_DOUBLE_IEEE128__ (i.e. when long double is __ieee128) it relies on __ibm128 and otherwise when __LONG_DOUBLE_IBM128__ (i.e. when long double is __ibm128) it relies on __ieee128.
Perhaps the former is true, if __ieee128 which is newer is supported, then __ibm128 will be hopefully too, but the latter might not neccessarily be (might be on GCC because the
__LONG_DOUBLE_IBM128__ macro is fairly new - from 2015).
So, shouldn't we use
# elif defined __LONG_DOUBLE_IBM128__ && defined __SIZEOF_FLOAT128__
instead of the current elif and maybe use __float128 instead of __ieee128 as tiny bit more portable thing?
I don't know if it will work that way with clang, because at least on F33 x86_64 I'm getting
clang --target=ppc64le-linux -mfloat128 -E -dD -xc /dev/null | grep FLOAT
error: option '-mfloat128' cannot be specified with 'ppc64le'

Comment 19 Jonathan Wakely 2021-03-21 12:54:25 UTC
(In reply to Jonathan Wakely from comment #17)
> That means the -mabi=ibmlongdouble -mno-float128 ABI won't be supported in
> libstdc++ with Clang.

Well, it will be supported, but maybe not interchangeable with -mabi=ieeelongdouble code. Or maybe since libstdc++.so itself is built with support for both types, it will actually just work.

Comment 20 Jonathan Wakely 2021-03-21 12:57:31 UTC
(In reply to Jakub Jelinek from comment #18)
> So, shouldn't we use
> # elif defined __LONG_DOUBLE_IBM128__ && defined __SIZEOF_FLOAT128__

OK, that's an easy change.

> instead of the current elif and maybe use __float128 instead of __ieee128 as
> tiny bit more portable thing?

Yes as suggested in comment 3. I think that's worth doing, even if it isn't sufficient on its own.

> I don't know if it will work that way with clang, because at least on F33
> x86_64 I'm getting
> clang --target=ppc64le-linux -mfloat128 -E -dD -xc /dev/null | grep FLOAT
> error: option '-mfloat128' cannot be specified with 'ppc64le'

?!?!

Comment 21 serge_sans_paille 2021-03-22 08:47:04 UTC
The relevant part in clang is

```
  if (LongDoubleWidth == 128) {
    Builder.defineMacro("__LONG_DOUBLE_128__");
    Builder.defineMacro("__LONGDOUBLE128");
    if (Opts.PPCIEEELongDouble)
      Builder.defineMacro("__LONG_DOUBLE_IEEE128__");
    else
      Builder.defineMacro("__LONG_DOUBLE_IBM128__");
  }
```

Comment 22 Jakub Jelinek 2021-03-22 08:59:07 UTC
Guess that is the part describing what long double is.
What the libstdc++ code wants to do is handle the compatibility by also supporting
the other floating point type, so if long double is IEEE754 quad, the __ibm128
type, if long double is IBM double double, the __ieee754 or __float128 type.
So, does clang have some macros that would be defined or have some particular value
if __ibm128 is supported and/or when __ieee754/__float128 is supported?
Unfortunately, seems even GCC doesn't.
On x86_64, we have
#define __SIZEOF_FLOAT80__ 16
#define __SIZEOF_FLOAT128__ 16
macros, but on ppc64le I don't see them :(.

Comment 23 Jakub Jelinek 2021-03-22 09:59:11 UTC
Note, seems gcc predefines __FLOAT128_TYPE__ when __ibm128 and __ieee128 are available and __FLOAT128__ when __float128 is available.

Comment 24 Jonathan Wakely 2021-03-23 16:27:14 UTC
I've committed a workaround to libstdc++ upstream:
https://gcc.gnu.org/g:baef0cffb58be7f5d9aeac6313ea9d8becc017b1

Comment 25 Dan Horák 2021-03-29 11:23:57 UTC
We can close this as there is a new gcc build with the libstdc++ workaround applied in both rawhide and  F-34, can't we?

Comment 26 Fedora Update System 2021-04-02 01:35:30 UTC
FEDORA-2021-a5a10edd89 has been pushed to the Fedora 34 testing repository.
Soon you'll be able to install the update with the following command:
`sudo dnf upgrade --enablerepo=updates-testing --advisory=FEDORA-2021-a5a10edd89`
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2021-a5a10edd89

See also https://fedoraproject.org/wiki/QA:Updates_Testing for more information on how to test updates.

Comment 27 Fedora Update System 2021-04-05 00:17:41 UTC
FEDORA-2021-a5a10edd89 has been pushed to the Fedora 34 stable repository.
If problem still persists, please make note of it in this bug report.

Comment 28 Tom Stellard 2021-04-13 04:47:16 UTC
*** Bug 1933252 has been marked as a duplicate of this bug. ***


Note You need to log in before you can comment on or make changes to this bug.