[X86][PPC] Support -mlong-double-64

[MaskRay]

Summary

-mlong-double-64 is supported on some ports of gcc (i386, x86_64, and ppc{32,64}).
On many other targets, there will be an error:

error: unrecognized command line option '-mlong-double-64'

This patch makes the driver option -mlong-double-64 available for x86
and ppc. The CC1 option -mlong-double-64 is available on all targets for
users to test on unsupported targets.

LongDoubleSize is added as a VALUE_LANGOPT so that the option can be
shared with -mlong-double-128 when we support it in clang.

Also, make powerpc*-linux-musl default to use 64-bit long double. It is
currently the only supported ABI on musl and is also how people
configure powerpc*-linux-musl-gcc.

[MaskRay]

Improve a test

[rnk]

Sounds good to me. I had a minor suggestion, though.

lib/Basic/Targets/PPC.cpp

468

I think it would be preferable to implement this in the base TargetInfo::adjust implementation, just to reduce code duplication. The driver already checks that the option makes sense for some particular target. I think it's actually kind of nice that the user can bypass the driver with -Xclang and try using it for some unsupported target to see what happens. It will probably work anyway.

[MaskRay]

Implement this in TargetInfo::adjust as rnk suggested

[hfinkel]

Can you please add a test ensuring that -malign-double and -mlong-double-64 interact properly? I think that, in the current patch, they do (as -malign-double is processed first), but I'd prefer that we cover that case explicitly.

[MaskRay]

Check -malign-double

[MaskRay]

Make powerpc* musl default to 64-bit long double

[andrew.w.kaylor]

In this review (https://reviews.llvm.org/D6260) @rsmith mentions that this should also have an effect on name mangling.

What will this do if the user calls a library function that expects a long double? What does gcc do in that case?

[rnk]

In D64067#1568533, @andrew.w.kaylor wrote:

In this review (https://reviews.llvm.org/D6260) @rsmith mentions that this should also have an effect on name mangling.

I'm not sure that's consistent with GCC, at least not anymore:
https://gcc.godbolt.org/z/eUviCd
Looks like you can still have an overload set with double and long double, even if they both use the same representation. This is a backend -m flag, after all, so that seems reasonable to me.

What will this do if the user calls a library function that expects a long double? What does gcc do in that case?

Looks like it passes according to the usual 64-bit IEEE double representation.

[hfinkel]

In D64067#1568888, @rnk wrote:

In D64067#1568533, @andrew.w.kaylor wrote:

In this review (https://reviews.llvm.org/D6260) @rsmith mentions that this should also have an effect on name mangling.

I'm not sure that's consistent with GCC, at least not anymore:
https://gcc.godbolt.org/z/eUviCd
Looks like you can still have an overload set with double and long double, even if they both use the same representation.

It has to work that way, because they're different, standard language-level types.

One thing to realize about these flags is that they're ABI-altering flags. If the user provides the flag to alter the platform defaults, this only works if the user also ensures that matches the ABI of the relevant system libraries that the compiler is using (e.g., because the user is explicitly linking with a suitable libc).

This is a backend -m flag, after all, so that seems reasonable to me.

What will this do if the user calls a library function that expects a long double? What does gcc do in that case?

Looks like it passes according to the usual 64-bit IEEE double representation.

[andrew.w.kaylor]

In D64067#1568895, @hfinkel wrote:

One thing to realize about these flags is that they're ABI-altering flags. If the user provides the flag to alter the platform defaults, this only works if the user also ensures that matches the ABI of the relevant system libraries that the compiler is using (e.g., because the user is explicitly linking with a suitable libc).

Right. That's what I was trying to figure out. Are we relying on the users of this option not to shoot themselves in the foot? It sounds like yes. I believe that's the way we handled it in icc also, but gcc and icc do a lot of sketchy things that we wouldn't want to do in clang. In this case I don't object to the sketchiness, just so everyone realizes that's what we're doing.

[MaskRay]

Add tests to check mangling of long double

-mlong-double-64 has no effect on gcc x86.
On gcc ppc, -mlong-double-64 changes the mangled type from 'g' (ibmlongdouble) to 'e'

[MaskRay]

% g++ a.cc -S -o - | grep '^_Z3foo'
_Z3fooe:
% g++ a.cc -S -o - -mlong-double-64 | grep '^_Z3foo'
_Z3fooe:
% g++ a.cc -S -o - -mlong-double-128 | grep '^_Z3foo'
_Z3foog:

% powerpc64le-linux-gnu-g++ a.cc -S -o - -mlong-double-64 | grep '^_Z3foo'
_Z3fooe:
% powerpc64le-linux-gnu-g++ a.cc -S -o - -mlong-double-128 -mabi=ibmlongdouble | grep '^_Z3foo'
_Z3foog:
% powerpc64le-linux-gnu-g++ a.cc -S -o - -mlong-double-128 -mabi=ieeelongdouble | grep '^_Z3foo'
_Z3fooU10__float128:

With this patch, the mangling scheme of -mlong-double-64 is consistent with gcc on x86 and ppc. (-mlong-double-128 is not supported now)

% clang a.cc -S -o - -mlong-double-64 | grep '^_Z3foo'        
_Z3fooe:                                # @_Z3fooe
% clang a.cc -S -o - | grep '^_Z3foo'        
_Z3fooe:                                # @_Z3fooe

% clang -target ppc64le a.cc -S -o - -mlong-double-64 | grep '^_Z3foo'
_Z3fooe:                                # @_Z3fooe
% clang -target ppc64le a.cc -S -o - | grep '^_Z3foo'  # corresponds to gcc -mlong-double-128 -mabi=ibmlongdouble
_Z3foog:                                # @_Z3foog

[MaskRay]

Add more tests: elf32/elf64/darwin...

[rnk]

lgtm

[troyj]

Hi, we just inherited this commit at Cray when we did our latest upstream merge and there are a few problems with it that I'd like to point out. Sorry that I was not part of the initial discussion here, but I didn't know that this work was being done and I had already done it for x86 in our downstream compiler a while ago.

1. This patch adds the -m options to f_Group, which is weird. They should be in m_Group since they are target-specific. I actually implemented them as part of a subgroup of m_Group so that I can take the last option from that group that appears on the command line.
2. This patch lacks the GNU -mlong-double-80 option for x86.
3. Instead of tracking the size in clang/Basic/LangOptions.def, I track it in clang/Basic/TargetOptions.h. This is a target-specific thing afterall.

We're trying to resolve merge conflicts with this change and might be able to submit a patch to help reduce our differences. Would that be of interest?

[hfinkel]

In D64067#1592201, @troyj wrote:

Hi, we just inherited this commit at Cray when we did our latest upstream merge and there are a few problems with it that I'd like to point out. Sorry that I was not part of the initial discussion here, but I didn't know that this work was being done and I had already done it for x86 in our downstream compiler a while ago.

1. This patch adds the -m options to f_Group, which is weird. They should be in m_Group since they are target-specific. I actually implemented them as part of a subgroup of m_Group so that I can take the last option from that group that appears on the command line.
2. This patch lacks the GNU -mlong-double-80 option for x86.
3. Instead of tracking the size in clang/Basic/LangOptions.def, I track it in clang/Basic/TargetOptions.h. This is a target-specific thing afterall.

We're trying to resolve merge conflicts with this change and might be able to submit a patch to help reduce our differences. Would that be of interest?

Yes, that would be helpful.

[rnk]

In D64067#1592201, @troyj wrote:

Hi, we just inherited this commit at Cray when we did our latest upstream merge and there are a few problems with it that I'd like to point out. Sorry that I was not part of the initial discussion here, but I didn't know that this work was being done and I had already done it for x86 in our downstream compiler a while ago.

1. This patch adds the -m options to f_Group, which is weird. They should be in m_Group since they are target-specific. I actually implemented them as part of a subgroup of m_Group so that I can take the last option from that group that appears on the command line.

Oops, seems like a bug.

2. This patch lacks the GNU -mlong-double-80 option for x86.
3. Instead of tracking the size in clang/Basic/LangOptions.def, I track it in clang/Basic/TargetOptions.h. This is a target-specific thing afterall.

Sema needs to know about this option for sizeof(long double). I believe that's the guiding line between the various option kinds. However, it means almost everything ends up being a language option, for better or worse.

We're trying to resolve merge conflicts with this change and might be able to submit a patch to help reduce our differences. Would that be of interest?

Sounds good.

include/clang/Basic/LangOptions.def

 COMPATIBLE_LANGOPT(Optimize          , 1, 0, "__OPTIMIZE__ predefined macro")
 COMPATIBLE_LANGOPT(OptimizeSize      , 1, 0, "__OPTIMIZE_SIZE__ predefined macro")
 COMPATIBLE_LANGOPT(Static            , 1, 0, "__STATIC__ predefined macro (as opposed to __DYNAMIC__)")
 VALUE_LANGOPT(PackStruct  , 32, 0,
               "default struct packing maximum alignment")
 VALUE_LANGOPT(MaxTypeAlign  , 32, 0,
               "default maximum alignment for types")
 VALUE_LANGOPT(AlignDouble            , 1, 0, "Controls if doubles should be aligned to 8 bytes (x86 only)")
+VALUE_LANGOPT(LongDoubleSize        , 32, 0, "width of long double")
 COMPATIBLE_VALUE_LANGOPT(PICLevel    , 2, 0, "__PIC__ level")
 COMPATIBLE_VALUE_LANGOPT(PIE         , 1, 0, "is pie")
 LANGOPT(ROPI                         , 1, 0, "Read-only position independence")
 LANGOPT(RWPI                         , 1, 0, "Read-write position independence")
 COMPATIBLE_LANGOPT(GNUInline         , 1, 0, "GNU inline semantics")
 COMPATIBLE_LANGOPT(NoInlineDefine    , 1, 0, "__NO_INLINE__ predefined macro")
 COMPATIBLE_LANGOPT(Deprecated        , 1, 0, "__DEPRECATED predefined macro")
 COMPATIBLE_LANGOPT(FastMath          , 1, 0, "fast FP math optimizations, and __FAST_MATH__ predefined macro")

include/clang/Driver/Options.td

def miamcu : Flag<["-"], "miamcu">, Group<m_Group>, Flags<[DriverOption, CoreOption]>,
   HelpText<"Use Intel MCU ABI">;
 def mno_iamcu : Flag<["-"], "mno-iamcu">, Group<m_Group>, Flags<[DriverOption, CoreOption]>;
 def malign_functions_EQ : Joined<["-"], "malign-functions=">, Group<clang_ignored_m_Group>;
 def malign_loops_EQ : Joined<["-"], "malign-loops=">, Group<clang_ignored_m_Group>;
 def malign_jumps_EQ : Joined<["-"], "malign-jumps=">, Group<clang_ignored_m_Group>;
 def mfancy_math_387 : Flag<["-"], "mfancy-math-387">, Group<clang_ignored_m_Group>;
 def mlong_calls : Flag<["-"], "mlong-calls">, Group<m_Group>,
   HelpText<"Generate branches with extended addressability, usually via indirect jumps.">;
+def mlong_double_64 : Flag<["-"], "mlong-double-64">, Group<f_Group>, Flags<[CC1Option]>,
+  HelpText<"Force long double to be 64 bits">;
 def mno_long_calls : Flag<["-"], "mno-long-calls">, Group<m_Group>,
   HelpText<"Restore the default behaviour of not generating long calls">;
 def mexecute_only : Flag<["-"], "mexecute-only">, Group<m_arm_Features_Group>,
   HelpText<"Disallow generation of data access to code sections (ARM only)">;
 def mno_execute_only : Flag<["-"], "mno-execute-only">, Group<m_arm_Features_Group>,
   HelpText<"Allow generation of data access to code sections (ARM only)">;
 def mtp_mode_EQ : Joined<["-"], "mtp=">, Group<m_arm_Features_Group>, Values<"soft,cp15,el0,el1,el2,el3">,
   HelpText<"Thread pointer access method (AArch32/AArch64 only)">;

lib/Basic/Targets/PPC.cpp

bool PPCTargetInfo::isValidCPUName(StringRef Name) const {
   return llvm::find(ValidCPUNames, Name) != std::end(ValidCPUNames);
 }
 
 void PPCTargetInfo::fillValidCPUList(SmallVectorImpl<StringRef> &Values) const {
   Values.append(std::begin(ValidCPUNames), std::end(ValidCPUNames));
 }
 
 void PPCTargetInfo::adjust(LangOptions &Opts) {
+  TargetInfo::adjust(Opts);
   if (HasAltivec)
     Opts.AltiVec = 1;
-  TargetInfo::adjust(Opts);
+  if (Opts.LongDoubleSize == 64) {
+    LongDoubleWidth = LongDoubleAlign = 64;
+    LongDoubleFormat = &llvm::APFloat::IEEEdouble();
+  }
 }
 
 ArrayRef<Builtin::Info> PPCTargetInfo::getTargetBuiltins() const {
   return llvm::makeArrayRef(BuiltinInfo, clang::PPC::LastTSBuiltin -
                                              Builtin::FirstTSBuiltin);
 }

lib/Basic/Targets/X86.h

#include "clang/Basic/X86Target.def"
   enum FPMathKind { FP_Default, FP_SSE, FP_387 } FPMath = FP_Default;
 
 public:
   X86TargetInfo(const llvm::Triple &Triple, const TargetOptions &)
       : TargetInfo(Triple) {
     LongDoubleFormat = &llvm::APFloat::x87DoubleExtended();
   }
 
+  void adjust(LangOptions &Opts) override;
+
   unsigned getFloatEvalMethod() const override {
     // X87 evaluates with 80 bits "long double" precision.
     return SSELevel == NoSSE ? 2 : 0;
   }
 
   ArrayRef<const char *> getGCCRegNames() const override;
 
   ArrayRef<TargetInfo::GCCRegAlias> getGCCRegAliases() const override {

lib/Basic/Targets/X86.cpp

 };
 
 } // namespace targets
 } // namespace clang
 
 using namespace clang;
 using namespace clang::targets;
 
+void X86TargetInfo::adjust(LangOptions &Opts) {
+  TargetInfo::adjust(Opts);
+  if (Opts.LongDoubleSize == 64) {
+    LongDoubleWidth = 64;
+    LongDoubleAlign = DoubleAlign;
+    LongDoubleFormat = &llvm::APFloat::IEEEdouble();
+  }
+}
+
 bool X86TargetInfo::setFPMath(StringRef Name) {
   if (Name == "387") {
     FPMath = FP_387;
     return true;
   }
   if (Name == "sse") {
     FPMath = FP_SSE;
     return true;

lib/Driver/ToolChains/Clang.cpp

Args.AddLastArg(CmdArgs, options::OPT_ffine_grained_bitfield_accesses,
                   options::OPT_fno_fine_grained_bitfield_accesses);
 
   // Handle segmented stacks.
   if (Args.hasArg(options::OPT_fsplit_stack))
     CmdArgs.push_back("-split-stacks");
 
   RenderFloatingPointOptions(TC, D, OFastEnabled, Args, CmdArgs);
 
+  if (Arg *A = Args.getLastArg(options::OPT_mlong_double_64)) {
+    if (TC.getArch() == llvm::Triple::x86 ||
+        TC.getArch() == llvm::Triple::x86_64 ||
+        TC.getArch() == llvm::Triple::ppc || TC.getTriple().isPPC64()) {
+      CmdArgs.push_back("-mlong-double-64");
+    } else {
+      D.Diag(diag::err_drv_unsupported_opt_for_target)
+          << A->getAsString(Args) << TripleStr;
+    }
+  }
+
   // Decide whether to use verbose asm. Verbose assembly is the default on
   // toolchains which have the integrated assembler on by default.
   bool IsIntegratedAssemblerDefault = TC.IsIntegratedAssemblerDefault();
   if (Args.hasFlag(options::OPT_fverbose_asm, options::OPT_fno_verbose_asm,
                    IsIntegratedAssemblerDefault) ||
       Args.hasArg(options::OPT_dA))
     CmdArgs.push_back("-masm-verbose");
 

lib/Frontend/CompilerInvocation.cpp

#include "clang/Frontend/LangStandards.def"
   Opts.ObjCDefaultSynthProperties =
     !Args.hasArg(OPT_disable_objc_default_synthesize_properties);
   Opts.EncodeExtendedBlockSig =
     Args.hasArg(OPT_fencode_extended_block_signature);
   Opts.EmitAllDecls = Args.hasArg(OPT_femit_all_decls);
   Opts.PackStruct = getLastArgIntValue(Args, OPT_fpack_struct_EQ, 0, Diags);
   Opts.MaxTypeAlign = getLastArgIntValue(Args, OPT_fmax_type_align_EQ, 0, Diags);
   Opts.AlignDouble = Args.hasArg(OPT_malign_double);
+  Opts.LongDoubleSize = Args.hasArg(OPT_mlong_double_64) ? 64 : 0;
   Opts.PICLevel = getLastArgIntValue(Args, OPT_pic_level, 0, Diags);
   Opts.ROPI = Args.hasArg(OPT_fropi);
   Opts.RWPI = Args.hasArg(OPT_frwpi);
   Opts.PIE = Args.hasArg(OPT_pic_is_pie);
   Opts.Static = Args.hasArg(OPT_static_define);
   Opts.DumpRecordLayoutsSimple = Args.hasArg(OPT_fdump_record_layouts_simple);
   Opts.DumpRecordLayouts = Opts.DumpRecordLayoutsSimple
                         || Args.hasArg(OPT_fdump_record_layouts);

test/CodeGen/long-double-x86.c

-// RUN: %clang_cc1 %s -emit-llvm -o - -triple=i686-apple-darwin9 | grep x86_fp80
-
-long double x = 0;
-int checksize[sizeof(x) == 16 ? 1 : -1];

test/CodeGen/ppc64-align-long-double.c

-// REQUIRES: powerpc-registered-target
-// RUN: %clang_cc1 -triple powerpc64-unknown-linux-gnu -emit-llvm -o - %s | FileCheck %s
-
-struct S {
-  double a;
-  long double b;
-};
-
-// CHECK: %struct.{{[a-zA-Z0-9]+}} = type { double, ppc_fp128 }
-
-long double test (struct S x)
-{
-  return x.b;
-}
-
-// CHECK: %{{[0-9]}} = load ppc_fp128, ppc_fp128* %{{[a-zA-Z0-9]+}}, align 16

test/CodeGen/ppc64-long-double.c

+// RUN: %clang_cc1 -triple powerpc64-linux-musl -emit-llvm -o - %s -mlong-double-64 | \
+// RUN:   FileCheck --check-prefix=FP64 %s
+// RUN: %clang_cc1 -triple powerpc64-linux-musl -emit-llvm -o - %s | \
+// RUN:   FileCheck --check-prefix=IBM128 %s
+
+struct S {
+  char a;
+  long double b;
+};
+
+// FP64: %struct.{{[a-zA-Z0-9]+}} = type { i8, double }
+// IBM128: %struct.{{[a-zA-Z0-9]+}} = type { i8, ppc_fp128 }
+
+long double load(struct S x) {
+  return x.b;
+}
+
+// FP64: %{{[0-9]}} = load double, double* %{{[a-zA-Z0-9]+}}, align 8
+// IBM128: %{{[0-9]}} = load ppc_fp128, ppc_fp128* %{{[a-zA-Z0-9]+}}, align 16

test/CodeGen/x86-long-double.c

+// RUN: %clang_cc1 %s -emit-llvm -o - -triple=i686-apple-darwin9 -mlong-double-64 | \
+// RUN:   FileCheck --check-prefix=FP64-X32 %s
+// RUN: %clang_cc1 %s -emit-llvm -o - -triple=i686-apple-darwin9 | \
+// RUN:   FileCheck --check-prefix=FP80 %s
+// RUN: %clang_cc1 %s -emit-llvm -o - -triple=x86_64-linux-musl -mlong-double-64 | \
+// RUN:   FileCheck --check-prefix=FP64-X64 %s
+// RUN: %clang_cc1 %s -emit-llvm -o - -triple=x86_64-linux-musl | \
+// RUN:   FileCheck --check-prefix=FP80 %s
+
+long double x = 0;
+int size = sizeof(x);
+
+// FP64-X32: @x = global double {{.*}}, align 4
+// FP64-X32: @size = global i32 8
+// FP64-X64: @x = global double {{.*}}, align 8
+// FP64-X64: @size = global i32 8
+// FP80: @x = global x86_fp80 {{.*}}, align 16
+// FP80: @size = global i32 16

test/Driver/mlong-double-64.c

+// RUN: %clang -target powerpc-linux-musl -c -### %s -mlong-double-64 2>&1 | FileCheck %s
+// RUN: %clang -target powerpc64-pc-freebsd12 -c -### %s -mlong-double-64 2>&1 | FileCheck %s
+// RUN: %clang -target powerpc64le-linux-musl -c -### %s -mlong-double-64 2>&1 | FileCheck %s
+// RUN: %clang -target i686-linux-gnu -c -### %s -mlong-double-64 2>&1 | FileCheck %s
+// RUN: %clang -target x86_64-linux-musl -c -### %s -mlong-double-64 2>&1 | FileCheck %s
+
+// CHECK: "-mlong-double-64"
+
+// RUN: %clang -target aarch64 -c -### %s -mlong-double-64 2>&1 | FileCheck --check-prefix=ERR %s
+
+// ERR: error: unsupported option '-mlong-double-64' for target 'aarch64'