Hi! On Tue, Nov 03, 2020 at 10:25:05PM -0500, Michael Meissner wrote: > On Sat, Oct 31, 2020 at 11:39:23PM +1030, Alan Modra wrote: > > 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.
Yeah. > It was more just a case for explicitly adding -mabi=ibmlongdouble for the > modules that need it, and letting the default option be used for the modules > that don't use IBM long doubles. > > Because we only have set of GNU attributes for an entire shared library, it is > important that all 3 cases for long double not set the attributes. > > I first took off the -mno-gnu-attributes options, and built libgcc with 3 > different compilers (long double = 64 bits, long double = IEEE 128-bits, and > long double = IBM 128-bit). The modules in the patch were the ones that set > gnu attribute #4 to 5 (i.e. use IBM 128-bit floating point). > > I then looked in the libgcc built with the configure option: > --without-long-double-128 > > and a bunch of modules showed setting gnu attribute #4 to 9 (i.e. use of long > double as double). > > The reason is within GCC if you compile with -mlong-double-64 (or build with a > compiler configured with --without-long-double-128), every time you use double > or _Complex double, it will set gnu attribute #4 to 5. It is due to this code > in rs6000_emit_move in rs6000.c: > > #ifdef HAVE_AS_GNU_ATTRIBUTE > /* If we use a long double type, set the flags in .gnu_attribute that say > what the long double type is. This is to allow the linker's warning > message for the wrong long double to be useful, even if the function does > not do a call (for example, doing a 128-bit add on power9 if the long > double type is IEEE 128-bit. Do not set this if __ibm128 or __floa128 > are > used if they aren't the default long dobule type. */ > if (rs6000_gnu_attr && (HAVE_LD_PPC_GNU_ATTR_LONG_DOUBLE || TARGET_64BIT)) > { > if (TARGET_LONG_DOUBLE_128 && (mode == TFmode || mode == TCmode)) > rs6000_passes_float = rs6000_passes_long_double = true; > > else if (!TARGET_LONG_DOUBLE_128 && (mode == DFmode || mode == DCmode)) > rs6000_passes_float = rs6000_passes_long_double = true; > } > #endif Yeah that is just bad. Fix that? Modes are not types. > The code in rs6000-call.c (init_cumulative_args and > rs6000_function_arg_advance_1) actually does look at the types and only sets > rs6000_passes_long_double if the type is the long double type. > > So I figured it was better just to build libgcc where we explicitly enable the > IBM floating point and just turn off gnu attributes globally in the library. It is much better to just fix the bug. Please try that, it is a real bug that needs real fixing no matter what, so might as well do it now (in a separate patch) and not have to work around it for the one place you know it to hurt now. > The code in rs6000_emit_move is perhaps well intentioned, but by looking at > just modes and not types, it can miss things. However, that is a giant can of > worms, and we can play whack-a-mole trying to get everything right. Perhaps > somebody (else) can tackle it. But I would prefer to just fix the issue at > hand, rather than possibly having an endless battle to get things right. It does very much the wrong thing now. It needs fixing. You need it fixed for this patch, to do this patch properly. Please just fix it. > As I recall, what the code in rs6000_emit_move is trying to do is to catch > places where you use long double, but no call is made. I.e. > > struct foo { > /* ... */ > long double ld; > /* ... */ > }; > > void bar (struct foo *p) > { > /* ... */ > (p->ld)++; > /* ... */ > } > > On a power9/10 system with IEEE long double or on a system with a 64-bit long > double, that code would not generate a call, but it does depend on the long > double format (and hence should set the gnu attribute #4). Yes, but setting it for any DFmode (with 64-bit long double) is just wrong. Segher