[PowerPC] Use IEEE long double in proper toolchain

[qiucf]

Summary

In toolchains with proper support for IEEE float128, we can switch default long double semantics to it, unless explicitly specified with options.

[jsji]

LGTM. Thanks for working on this!

[jsji]

clang/lib/Driver/ToolChains/PPCLinux.cpp

76

What if we have -mabi=elfv2 in command line?

[jsji]

Can we just update bool IEEELongDouble = false; to bool IEEELongDouble = T.isOSLinux(); in clang/lib/Driver/ToolChains/Clang.cpp?

[nemanjai]

I don't think this is the right approach. Personally, I would find it surprising if simply having GCC 12.1 on the system changes my long double default. Imagine if I simply install and use such a toolchain on a machine for whatever reason. If the default for such a toolchain is not IEEE, then the clang default diverges from the GCC toolchain default.

Would there be a way to query the GCC toolchain for what the default is for long double?

[jsji]

Would there be a way to query the GCC toolchain for what the default is for long double?

There would certainly be ways to query. But I don't think we should change the default *BASED* on the gcc toolchain installed.

As you mentioned, I would find it surprising if simply having GCC 12.1 on the system changes my long double default. Imagine if I simply install and use such a toolchain on a machine for whatever reason

So, the proposal is we change the default on Linux, so we will not change the default based on the gcc version, it is based on OS and clang version only.
but will emit warning if we detect that the GCC toolchain is too old.

Can we just update bool IEEELongDouble = false;  to bool IEEELongDouble = T.isOSLinux(); in clang/lib/Driver/ToolChains/Clang.cpp?

[nemanjai]

In D117181#3261396, @jsji wrote:

So, the proposal is we change the default on Linux, so we will not change the default based on the gcc version, it is based on OS and clang version only.
but will emit warning if we detect that the GCC toolchain is too old.

Can we just update bool IEEELongDouble = false;  to bool IEEELongDouble = T.isOSLinux(); in clang/lib/Driver/ToolChains/Clang.cpp?

Won't that end up producing a warning on ALL code built on any Linux distro with a GCC toolchain older than 12.1? That would be terrible.
Ultimately, what we care about is checking for the default for the distro toolchain. I am not sure what a good proxy for that is, but I don't think it is "all Linux distros".

[jsji]

Won't that end up producing a warning on ALL code built on any Linux distro with a GCC toolchain older than 12.1? That would be terrible.

Good point. Yes, so should be something like:

IsDistroWithNewToolchain = ( Distro.IsRedhat() && Distro >= Distro::RHEL9  ||  (Distro.IsUbuntu() && Distro >= Distro::xxx)
bool IEEELongDouble = T.isOSLinux() && IsDistroWithNewToolchain;

[qiucf]

In D117181#3262288, @jsji wrote:

Won't that end up producing a warning on ALL code built on any Linux distro with a GCC toolchain older than 12.1? That would be terrible.

Good point. Yes, so should be something like:

IsDistroWithNewToolchain = ( Distro.IsRedhat() && Distro >= Distro::RHEL9  ||  (Distro.IsUbuntu() && Distro >= Distro::xxx)
bool IEEELongDouble = T.isOSLinux() && IsDistroWithNewToolchain;

Thanks for reminding about checking Linux distro version! Besides, would it be acceptable that we add a variable to cmake to determine default long double semantics (like current GCC)?

[jsji]

Besides, would it be acceptable that we add a variable to cmake to determine default long double semantics (like current GCC)?

Yes, we can. But I think that can be in another patch, and we shouldn't rely on it -- as we shouldn't assume user will config/build clang/llvm on the target machine.

[qiucf]

Determining float ABI by system library without explicit options may cause confusion. Since Fedora has successfully switched using ieeelongdouble on ppc64le (https://developers.redhat.com/articles/2023/05/16/benefits-fedora-38-long-double-transition-ppc64le ), this patch is not so relevant.

clang/lib/Driver/ToolChains/PPCLinux.h

 public:
   PPCLinuxToolChain(const Driver &D, const llvm::Triple &Triple,
                     const llvm::opt::ArgList &Args);
 
   void
   AddClangSystemIncludeArgs(const llvm::opt::ArgList &DriverArgs,
                             llvm::opt::ArgStringList &CC1Args) const override;
 
+  void
+  addClangTargetOptions(const llvm::opt::ArgList &DriverArgs,
+                        llvm::opt::ArgStringList &CC1Args,
+                        Action::OffloadKind DeviceOffloadKind) const override;
+
 private:
   bool SupportIEEEFloat128(const Driver &D, const llvm::Triple &Triple,
                            const llvm::opt::ArgList &Args) const;
 };
 
 } // end namespace toolchains
 } // end namespace driver
 } // end namespace clang
 
 #endif // LLVM_CLANG_LIB_DRIVER_TOOLCHAINS_PPC_LINUX_H

clang/lib/Driver/ToolChains/PPCLinux.cpp

 //===-- PPCLinux.cpp - PowerPC ToolChain Implementations --------*- C++ -*-===//
 //
 // 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
 //
 //===----------------------------------------------------------------------===//
 
 #include "PPCLinux.h"
 #include "clang/Driver/Driver.h"
 #include "clang/Driver/DriverDiagnostic.h"
 #include "clang/Driver/Options.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Path.h"
 
+using namespace clang::driver;
 using namespace clang::driver::toolchains;
 using namespace llvm::opt;
 using namespace llvm::sys;
 
 // Glibc older than 2.32 doesn't fully support IEEE float128. Here we check
 // glibc version by looking at dynamic linker name.
 static bool GlibcSupportsFloat128(const std::string& Linker) {
   llvm::SmallVector<char, 16> Path;
if (!DriverArgs.hasArg(clang::driver::options::OPT_nostdinc) &&
     SmallString<128> P(D.ResourceDir);
     llvm::sys::path::append(P, "include", "ppc_wrappers");
     addSystemInclude(DriverArgs, CC1Args, P);
   }
 
   Linux::AddClangSystemIncludeArgs(DriverArgs, CC1Args);
 }
 
+void PPCLinuxToolChain::addClangTargetOptions(
+    const llvm::opt::ArgList &DriverArgs, llvm::opt::ArgStringList &CC1Args,
+    Action::OffloadKind DeviceOffloadKind) const {
+  if (SupportIEEEFloat128(getDriver(), getTriple(), DriverArgs) &&
+      !DriverArgs.hasArg(options::OPT_mabi_EQ))
+    CC1Args.push_back("-mabi=ieeelongdouble");
+}
+
 bool PPCLinuxToolChain::SupportIEEEFloat128(
     const Driver &D, const llvm::Triple &Triple,
     const llvm::opt::ArgList &Args) const {
   if (!Triple.isLittleEndian() || !Triple.isPPC64())
     return false;
 
   if (Args.hasArg(options::OPT_nostdlib, options::OPT_nostdlibxx))
     return true;
 
   bool HasUnsupportedCXXLib =
       ToolChain::GetCXXStdlibType(Args) == CST_Libcxx &&
       GCCInstallation.getVersion().isOlderThan(12, 1, 0);
 
   return GlibcSupportsFloat128(Linux::getDynamicLinker(Args)) &&
          !(D.CCCIsCXX() && HasUnsupportedCXXLib);
 }

clang/test/Driver/ppc-float-abi.cpp

 // REQUIRES: powerpc-registered-target
 // RUN: %clang -### --driver-mode=g++ -target powerpc64le-linux-gnu %s \
 // RUN:  --gcc-toolchain=%S/Inputs/powerpc64le-linux-gnu-tree/gcc-11.2.0 \
 // RUN:  -mabi=ieeelongdouble -stdlib=libstdc++ 2>&1 | FileCheck %s
 // RUN: %clang -### --driver-mode=g++ -target powerpc64le-linux-gnu %s \
 // RUN:  -mabi=ieeelongdouble -stdlib=libc++ 2>&1 | FileCheck %s
 // RUN: %clang -### --driver-mode=g++ -target powerpc64le-linux-gnu %s\
 // RUN:  -mabi=ieeelongdouble -stdlib=libc++ -Wno-unsupported-abi 2>&1 | \
 // RUN:  FileCheck %s --check-prefix=NOWARN
+// RUN: %clang -### -target powerpc64le-linux-gnu %s -nostdlib 2>&1 | \
+// RUN:  FileCheck %s --check-prefix=IEEE
+// RUN: %clang -### -target powerpc64le-linux-gnu %s -nostdlib \
+// RUN:  -mabi=ibmlongdouble 2>&1 | FileCheck %s --check-prefix=NOIEEE
 
 // CHECK: warning: float ABI 'ieeelongdouble' is not supported by current library
 // NOWARN-NOT: warning: float ABI 'ieeelongdouble' is not supported by current library
+// IEEE: -mabi=ieeelongdouble
+// NOIEEE-NOT: -mabi=ieeelongdouble
 long double foo(long double x) { return x;  }