[libcxx] Improve testing in #ifdef.

[Mordante]

Summary

Fixes midpoint error of PR51430.

[• Quuxplusone]

libcxx/test/std/numerics/numeric.ops/numeric.ops.midpoint/midpoint.float.pass.cpp

95

Who sets the __LONG_DOUBLE_128__ macro — some vendor's compiler?
Are you sure this doesn't need to be

#if defined(__PPC__) && (defined(__LONG_DOUBLE_128__) && __LONG_DOUBLE_128__) && !(defined(__LONG_DOUBLE_IEEE128__) && __LONG_DOUBLE_IEEE128__)

to deal with the (hypothetical but I-don't-know-if-impossible) case of either __LONG_DOUBLE_128__ or __LONG_DOUBLE_IEEE128__ being defined as 0?

[Mordante]

libcxx/test/std/numerics/numeric.ops/numeric.ops.midpoint/midpoint.float.pass.cpp

95

Due to the bug report is for the release I strongly suspect the compiler in question is Clang.

Based on Clang's implementation I think the current approach is correct.
clang/lib/Basic/Targets/PPC.cpp:297

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__");
}

I still would like @Conanap to validate this fixes that build error on PPC.

[ldionne]

Can you link to the bug report? LGTM, thanks a lot for handling this.

[Conanap]

Sorry for the late reply! There were some internal discussions before we came to a conclusion.

While this patch works, __LONG_DOUBLE_128__ should be defined (or at least we haven't hit a case where it's undefined), so we would prefer something similar:

#if defined(__PPC__) && __LONG_DOUBLE_128__ && (!defined(__LONG_DOUBLE_IEEE128__) || !__LONG_DOUBLE_IEEE128__)

The compiler in question is indeed Clang.

I have a patch of the preferred implementation here: https://reviews.llvm.org/D108352

Thanks for the help!

[Mordante]

@Conanap I'm a bit confused now. Do you want me to fix this patch or do you want to land your patch?

[Conanap]

In D108071#2954864, @Mordante wrote:

@Conanap I'm a bit confused now. Do you want me to fix this patch or do you want to land your patch?

My patched is preferred here, thanks! I appreciate the work you put in though.

[Mordante]

In D108071#2954872, @Conanap wrote:

In D108071#2954864, @Mordante wrote:

@Conanap I'm a bit confused now. Do you want me to fix this patch or do you want to land your patch?

My patched is preferred here, thanks! I appreciate the work you put in though.

No problem, but next time it would be easier to just add review comments to an existing review instead of creating a new review.

libcxx/test/std/numerics/numeric.ops/numeric.ops.midpoint/midpoint.float.pass.cpp

 //===----------------------------------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
 // UNSUPPORTED: c++03, c++11, c++14, c++17

 
 //  Check two values "close to each other"
     T d1 = T(3.14);
     T d0 = std::nextafter(d1, T(2));
     T d2 = std::nextafter(d1, T(5));
     assert(d0 < d1);  // sanity checking
     assert(d1 < d2);  // sanity checking
 
-#if defined(__PPC__) && __LONG_DOUBLE_128__ && !__LONG_DOUBLE_IEEE128__
+#if defined(__PPC__) && defined(__LONG_DOUBLE_128__) && !defined(__LONG_DOUBLE_IEEE128__)
 //  For 128 bit long double implemented as 2 doubles on PowerPC,
 //  nextafterl() of libm gives imprecise results which fails the
 //  midpoint() tests below. So skip the test for this case.
     if constexpr (sizeof(T) != 16)
 #endif
     {
   //  Since there's nothing in between, the midpoint has to be one or the other
     T res;