[Power PC] add soft float support for ppc32

[spetrovic]

Summary

This patch enables soft float support for ppc32 architecture and fixes the ABI for variadic functions. This is first in set of patches for soft float support in LLVM.

[alexr]

PowerPC has floating point hardware by definition. Is this some new variant?

[annulen]

02.10.2015, 00:00, "Alex Rosenberg via cfe-commits" <[e-mail]>:

alexr added a subscriber: alexr.
alexr added a comment.

PowerPC has floating point hardware by definition. Is this some new variant?

Some cores from ppc400 series do not have hardware FP (for example, ppc 405)

[hfinkel]

include/clang/Basic/TargetInfo.h

688 ↗

(On Diff #36763)

Instead of adding this function, please use the same mechanism as X86_32TargetCodeGenInfo and X86_32ABIInfo to feed the soft-float abi information through.

lib/Basic/Targets.cpp

877 ↗

(On Diff #36763)

Add spaces after { and before }.

1072 ↗

(On Diff #36763)

This check can be part of the loop above.

lib/Driver/Tools.cpp

1372 ↗

(On Diff #36763)

Unless there is a good reason to consider all unknown strings equivalent to "hard", please produce an error (and an associated test case).

test/Driver/ppc-features.cpp

18 ↗

(On Diff #36763)

Also add a test case with -mhard-float, and both -msoft-float and -mhard-float in different orders.
Also add test cases with -mfloat-abi=X

[spetrovic]

Comments addressed.

[spetrovic]

added test case

[spetrovic]

ping

[hfinkel]

Can you please make sure we produce some sensible error should someone try to use soft float with ppc64 (since we don't support that), and add an associated test.

lib/Driver/Tools.h

739 ↗

(On Diff #38888)

We don't seem to support (or even accept) SoftFP. Given that, either remove it from the enum or add support for it.

742 ↗

(On Diff #38888)

Unindent this and ppc::FloatABI -> FloatABI

[spetrovic]

Comments addressed.

[spetrovic]

ping

[hfinkel]

lib/CodeGen/TargetInfo.cpp

3421 ↗

(On Diff #40143)

Line too long.

lib/Driver/Tools.cpp

1353 ↗

(On Diff #40143)

Remove this FIXME. It is not clear this will ever be supported (is there actually ppc64 hardware without an FP unit)?

1362 ↗

(On Diff #40143)

Don't introduce a new error for this. Just reuse the diag::err_drv_invalid_mfloat_abi error.

[spetrovic]

lib/Driver/Tools.cpp

1439 ↗

(On Diff #41736)

I'm planning to support soft float for ppc64. At this moment I don't know name of ppc64 hardware without FP unit. I know that gcc supports soft float for ppc64.

[spetrovic]

ping

[hfinkel]

One minor comment, otherwise LGTM.

lib/Driver/Tools.cpp

1477 ↗

(On Diff #41736)

Don't need the { } here.

[rjmccall]

lib/Driver/Tools.cpp

1477 ↗

(On Diff #41736)

We don't seem to have a specific style guideline *mandating* the uses of braces around even single-line statements, but please don't *discourage* them.

[hfinkel]

lib/Driver/Tools.cpp

1477 ↗

(On Diff #41736)

I don't believe this represents the current consensus convention on this matter. We do actively discourage use of unnecessary braces for ifs, fors, etc. The exception being that a series of ifs and elses should have braces for all of them if any of them need them.

I often point people at the examples here as representative: http://llvm.org/docs/CodingStandards.html#don-t-use-else-after-a-return

You're right, however, we don't formally dictate this policy in our coding standards. Do you actively dislike this viewpoint?

[rjmccall]

lib/Driver/Tools.cpp

1477 ↗

(On Diff #41736)

The coding style document uses both styles fairly interchangeably; I would not say it states a consensus. And yes, I actively encourage people to add braces in code reviews.

[hfinkel]

lib/Driver/Tools.cpp

1477 ↗

(On Diff #41736)

Fair enough; Strahinja, please keep the braces :-)

cfe/trunk/lib/CodeGen/TargetInfo.cpp

return emitVoidPtrVAArg(CGF, VAListAddr, Ty, /*indirect*/ false,
                           CharUnits::fromQuantity(8),
                           /*allowHigherAlign*/ false);
 }
 
 // PowerPC-32
 namespace {
 /// PPC32_SVR4_ABIInfo - The 32-bit PowerPC ELF (SVR4) ABI information.
 class PPC32_SVR4_ABIInfo : public DefaultABIInfo {
+bool IsSoftFloatABI;
 public:
-  PPC32_SVR4_ABIInfo(CodeGen::CodeGenTypes &CGT) : DefaultABIInfo(CGT) {}
+  PPC32_SVR4_ABIInfo(CodeGen::CodeGenTypes &CGT, bool SoftFloatABI)
+      : DefaultABIInfo(CGT), IsSoftFloatABI(SoftFloatABI) {}
 
   Address EmitVAArg(CodeGenFunction &CGF, Address VAListAddr,
                     QualType Ty) const override;
 };
 
 class PPC32TargetCodeGenInfo : public TargetCodeGenInfo {
 public:
-  PPC32TargetCodeGenInfo(CodeGenTypes &CGT)
-      : TargetCodeGenInfo(new PPC32_SVR4_ABIInfo(CGT)) {}
+  PPC32TargetCodeGenInfo(CodeGenTypes &CGT, bool SoftFloatABI)
+      : TargetCodeGenInfo(new PPC32_SVR4_ABIInfo(CGT, SoftFloatABI)) {}
 
   int getDwarfEHStackPointer(CodeGen::CodeGenModule &M) const override {
     // This is recovered from gcc output.
     return 1; // r1 is the dedicated stack pointer
   }
 
   bool initDwarfEHRegSizeTable(CodeGen::CodeGenFunction &CGF,
                                llvm::Value *Address) const override;
Address PPC32_SVR4_ABIInfo::EmitVAArg(CodeGenFunction &CGF, Address VAList,
   //   unsigned short reserved;
   //   void *overflow_arg_area;
   //   void *reg_save_area;
   // };
 
   bool isI64 = Ty->isIntegerType() && getContext().getTypeSize(Ty) == 64;
   bool isInt =
       Ty->isIntegerType() || Ty->isPointerType() || Ty->isAggregateType();
+  bool isF64 = Ty->isFloatingType() && getContext().getTypeSize(Ty) == 64;
 
   // All aggregates are passed indirectly?  That doesn't seem consistent
   // with the argument-lowering code.
   bool isIndirect = Ty->isAggregateType();
 
   CGBuilderTy &Builder = CGF.Builder;
 
   // The calling convention either uses 1-2 GPRs or 1 FPR.
   Address NumRegsAddr = Address::invalid();
-  if (isInt) {
+  if (isInt || IsSoftFloatABI) {
     NumRegsAddr = Builder.CreateStructGEP(VAList, 0, CharUnits::Zero(), "gpr");
   } else {
     NumRegsAddr = Builder.CreateStructGEP(VAList, 1, CharUnits::One(), "fpr");
   }
 
   llvm::Value *NumRegs = Builder.CreateLoad(NumRegsAddr, "numUsedRegs");
 
   // "Align" the register count when TY is i64.
-  if (isI64) {
+  if (isI64 || (isF64 && IsSoftFloatABI)) {
     NumRegs = Builder.CreateAdd(NumRegs, Builder.getInt8(1));
     NumRegs = Builder.CreateAnd(NumRegs, Builder.getInt8((uint8_t) ~1U));
   }
 
   llvm::Value *CC =
       Builder.CreateICmpULT(NumRegs, Builder.getInt8(8), "cond");
 
   llvm::BasicBlock *UsingRegs = CGF.createBasicBlock("using_regs");
Address RegAddr = Address::invalid();
 
     Address RegSaveAreaPtr =
       Builder.CreateStructGEP(VAList, 4, CharUnits::fromQuantity(8));
     RegAddr = Address(Builder.CreateLoad(RegSaveAreaPtr),
                       CharUnits::fromQuantity(8));
     assert(RegAddr.getElementType() == CGF.Int8Ty);
 
     // Floating-point registers start after the general-purpose registers.
-    if (!isInt) {
+    if (!(isInt || IsSoftFloatABI)) {
       RegAddr = Builder.CreateConstInBoundsByteGEP(RegAddr,
                                                    CharUnits::fromQuantity(32));
     }
 
     // Get the address of the saved value by scaling the number of
     // registers we've used by the number of 
-    CharUnits RegSize = CharUnits::fromQuantity(isInt ? 4 : 8);
+    CharUnits RegSize = CharUnits::fromQuantity((isInt || IsSoftFloatABI) ? 4 : 8);
     llvm::Value *RegOffset =
       Builder.CreateMul(NumRegs, Builder.getInt8(RegSize.getQuantity()));
     RegAddr = Address(Builder.CreateInBoundsGEP(CGF.Int8Ty,
                                             RegAddr.getPointer(), RegOffset),
                       RegAddr.getAlignment().alignmentOfArrayElement(RegSize));
     RegAddr = Builder.CreateElementBitCast(RegAddr, DirectTy);
 
     // Increase the used-register count.
-    NumRegs = Builder.CreateAdd(NumRegs, Builder.getInt8(isI64 ? 2 : 1));
+    NumRegs =
+      Builder.CreateAdd(NumRegs, 
+                        Builder.getInt8((isI64 || (isF64 && IsSoftFloatABI)) ? 2 : 1));
     Builder.CreateStore(NumRegs, NumRegsAddr);
 
     CGF.EmitBranch(Cont);
   }
 
   // Case 2: consume space in the overflow area.
   Address MemAddr = Address::invalid();
   {
case llvm::Triple::thumbeb:
                (CodeGenOpts.FloatABI != "soft" &&
                 Triple.getEnvironment() == llvm::Triple::GNUEABIHF))
         Kind = ARMABIInfo::AAPCS_VFP;
 
       return *(TheTargetCodeGenInfo = new ARMTargetCodeGenInfo(Types, Kind));
     }
 
   case llvm::Triple::ppc:
-    return *(TheTargetCodeGenInfo = new PPC32TargetCodeGenInfo(Types));
+    return *(TheTargetCodeGenInfo = 
+             new PPC32TargetCodeGenInfo(Types, CodeGenOpts.FloatABI == "soft"));
   case llvm::Triple::ppc64:
     if (Triple.isOSBinFormatELF()) {
       PPC64_SVR4_ABIInfo::ABIKind Kind = PPC64_SVR4_ABIInfo::ELFv1;
       if (getTarget().getABI() == "elfv2")
         Kind = PPC64_SVR4_ABIInfo::ELFv2;
       bool HasQPX = getTarget().getABI() == "elfv1-qpx";
 
       return *(TheTargetCodeGenInfo =

cfe/trunk/lib/Driver/Tools.h

enum class FloatABI {
   Soft,
   SoftFP,
   Hard,
 };
 
 FloatABI getARMFloatABI(const ToolChain &TC, const llvm::opt::ArgList &Args);
 } // end namespace arm
 
+namespace ppc {
+enum class FloatABI {
+  Invalid,
+  Soft,
+  Hard,
+};
+
+FloatABI getPPCFloatABI(const Driver &D, const llvm::opt::ArgList &Args);
+} // end namespace ppc
+
 namespace XCore {
 // For XCore, we do not need to instantiate tools for PreProcess, PreCompile and
 // Compile.
 // We simply use "clang -cc1" for those actions.
 class LLVM_LIBRARY_VISIBILITY Assembler : public Tool {
 public:
   Assembler(const ToolChain &TC) : Tool("XCore::Assembler", "XCore-as", TC) {}
 

cfe/trunk/lib/Driver/Tools.cpp

return llvm::StringSwitch<const char *>(CPUName)
         .Case("powerpc64", "ppc64")
         .Case("powerpc64le", "ppc64le")
         .Default("");
   }
 
   return "";
 }
 
-static void getPPCTargetFeatures(const ArgList &Args,
+static void getPPCTargetFeatures(const Driver &D, const llvm::Triple &Triple,
+                                 const ArgList &Args,
                                  std::vector<const char *> &Features) {
   for (const Arg *A : Args.filtered(options::OPT_m_ppc_Features_Group)) {
     StringRef Name = A->getOption().getName();
     A->claim();
 
     // Skip over "-m".
     assert(Name.startswith("m") && "Invalid feature name.");
     Name = Name.substr(1);
 
     bool IsNegative = Name.startswith("no-");
     if (IsNegative)
       Name = Name.substr(3);
 
     // Note that gcc calls this mfcrf and LLVM calls this mfocrf so we
     // pass the correct option to the backend while calling the frontend
     // option the same.
     // TODO: Change the LLVM backend option maybe?
     if (Name == "mfcrf")
       Name = "mfocrf";
 
     Features.push_back(Args.MakeArgString((IsNegative ? "-" : "+") + Name));
   }
 
+  ppc::FloatABI FloatABI = ppc::getPPCFloatABI(D, Args);
+  if (FloatABI == ppc::FloatABI::Soft &&
+      !(Triple.getArch() == llvm::Triple::ppc64 ||
+        Triple.getArch() == llvm::Triple::ppc64le))
+    Features.push_back("+soft-float");
+  else if (FloatABI == ppc::FloatABI::Soft &&
+           (Triple.getArch() == llvm::Triple::ppc64 ||
+            Triple.getArch() == llvm::Triple::ppc64le))
+    D.Diag(diag::err_drv_invalid_mfloat_abi) 
+        << "soft float is not supported for ppc64";
+
   // Altivec is a bit weird, allow overriding of the Altivec feature here.
   AddTargetFeature(Args, Features, options::OPT_faltivec,
                    options::OPT_fno_altivec, "altivec");
 }
 
+ppc::FloatABI ppc::getPPCFloatABI(const Driver &D, const ArgList &Args) {
+  ppc::FloatABI ABI = ppc::FloatABI::Invalid;
+  if (Arg *A =
+          Args.getLastArg(options::OPT_msoft_float, options::OPT_mhard_float,
+                          options::OPT_mfloat_abi_EQ)) {
+    if (A->getOption().matches(options::OPT_msoft_float))
+      ABI = ppc::FloatABI::Soft;
+    else if (A->getOption().matches(options::OPT_mhard_float))
+      ABI = ppc::FloatABI::Hard;
+    else {
+      ABI = llvm::StringSwitch<ppc::FloatABI>(A->getValue())
+                .Case("soft", ppc::FloatABI::Soft)
+                .Case("hard", ppc::FloatABI::Hard)
+                .Default(ppc::FloatABI::Invalid);
+      if (ABI == ppc::FloatABI::Invalid && !StringRef(A->getValue()).empty()) {
+        D.Diag(diag::err_drv_invalid_mfloat_abi) << A->getAsString(Args);
+        ABI = ppc::FloatABI::Hard;
+      }
+    }
+  }
+
+  // If unspecified, choose the default based on the platform.
+  if (ABI == ppc::FloatABI::Invalid) {
+    ABI = ppc::FloatABI::Hard;
+  }
+
+  return ABI;
+}
+
 void Clang::AddPPCTargetArgs(const ArgList &Args,
                              ArgStringList &CmdArgs) const {
   // Select the ABI to use.
   const char *ABIName = nullptr;
   if (getToolChain().getTriple().isOSLinux())
     switch (getToolChain().getArch()) {
     case llvm::Triple::ppc64: {
       // When targeting a processor that supports QPX, or if QPX is
void Clang::AddPPCTargetArgs(const ArgList &Args,
 
   if (Arg *A = Args.getLastArg(options::OPT_mabi_EQ))
     // The ppc64 linux abis are all "altivec" abis by default. Accept and ignore
     // the option if given as we don't have backend support for any targets
     // that don't use the altivec abi.
     if (StringRef(A->getValue()) != "altivec")
       ABIName = A->getValue();
 
+  ppc::FloatABI FloatABI =
+      ppc::getPPCFloatABI(getToolChain().getDriver(), Args);
+
+  if (FloatABI == ppc::FloatABI::Soft) {
+    // Floating point operations and argument passing are soft.
+    CmdArgs.push_back("-msoft-float");
+    CmdArgs.push_back("-mfloat-abi");
+    CmdArgs.push_back("soft");
+  } else {
+    // Floating point operations and argument passing are hard.
+    assert(FloatABI == ppc::FloatABI::Hard && "Invalid float abi!");
+    CmdArgs.push_back("-mfloat-abi");
+    CmdArgs.push_back("hard");
+  }
+
   if (ABIName) {
     CmdArgs.push_back("-target-abi");
     CmdArgs.push_back(ABIName);
   }
 }
 
 bool ppc::hasPPCAbiArg(const ArgList &Args, const char *Value) {
   Arg *A = Args.getLastArg(options::OPT_mabi_EQ);
static void getTargetFeatures(const ToolChain &TC, const llvm::Triple &Triple,
   case llvm::Triple::thumb:
   case llvm::Triple::thumbeb:
     getARMTargetFeatures(TC, Triple, Args, Features, ForAS);
     break;
 
   case llvm::Triple::ppc:
   case llvm::Triple::ppc64:
   case llvm::Triple::ppc64le:
-    getPPCTargetFeatures(Args, Features);
+    getPPCTargetFeatures(D, Triple, Args, Features);
     break;
   case llvm::Triple::systemz:
     getSystemZTargetFeatures(Args, Features);
     break;
   case llvm::Triple::aarch64:
   case llvm::Triple::aarch64_be:
     getAArch64TargetFeatures(D, Args, Features);
     break;

cfe/trunk/test/CodeGen/ppc-sfvarargs.c

+// RUN: %clang -O0 --target=powerpc-unknown-linux-gnu -EB -msoft-float -S -emit-llvm %s -o - | FileCheck %s
+
+#include <stdarg.h>
+void test(char *fmt, ...) {
+  va_list ap;
+  va_start(ap, fmt);
+  va_arg(ap, double);
+  va_end(ap);
+}
+
+void foo() {
+  double a;
+  test("test",a);
+}
+// CHECK: %{{[0-9]+}} = add i8 %numUsedRegs, 1
+// CHECK: %{{[0-9]+}} = and i8 %{{[0-9]+}}, -2
+// CHECK: %{{[0-9]+}} = mul i8 %{{[0-9]+}}, 4
 No newline at end of file

cfe/trunk/test/Driver/ppc-features.cpp

 // Check that we error when -faltivec is specified on non-ppc platforms.
 
 // RUN: %clang -target powerpc-unk-unk -faltivec -fsyntax-only %s
 // RUN: %clang -target powerpc64-linux-gnu -faltivec -fsyntax-only %s
 // RUN: %clang -target powerpc64-linux-gnu -maltivec -fsyntax-only %s
 
 // RUN: not %clang -target i386-pc-win32 -faltivec -fsyntax-only %s 2>&1 | FileCheck %s
 // RUN: not %clang -target x86_64-unknown-freebsd -faltivec -fsyntax-only %s 2>&1 | FileCheck %s
 // RUN: not %clang -target armv6-apple-darwin -faltivec -fsyntax-only %s 2>&1 | FileCheck %s
 // RUN: not %clang -target armv7-apple-darwin -faltivec -fsyntax-only %s 2>&1 | FileCheck %s
 // RUN: not %clang -target mips-linux-gnu -faltivec -fsyntax-only %s 2>&1 | FileCheck %s
 // RUN: not %clang -target mips64-linux-gnu -faltivec -fsyntax-only %s 2>&1 | FileCheck %s
 // RUN: not %clang -target sparc-unknown-solaris -faltivec -fsyntax-only %s 2>&1 | FileCheck %s
 
+// check -msoft-float option for ppc32
+// RUN: %clang -target powerpc-unknown-linux-gnu %s -msoft-float -### -o %t.o 2>&1 | FileCheck --check-prefix=CHECK-SOFTFLOAT %s
+// CHECK-SOFTFLOAT: "-target-feature" "+soft-float"
+
+// check -mfloat-abi=soft option for ppc32
+// RUN: %clang -target powerpc-unknown-linux-gnu %s -mfloat-abi=soft -### -o %t.o 2>&1 | FileCheck --check-prefix=CHECK-FLOATABISOFT %s
+// CHECK-FLOATABISOFT: "-target-feature" "+soft-float"
+
+// check -mhard-float option for ppc32
+// RUN: %clang -target powerpc-unknown-linux-gnu %s -mhard-float -### -o %t.o 2>&1 | FileCheck --check-prefix=CHECK-HARDFLOAT %s
+// CHECK-HARDFLOAT-NOT: "-target-feature" "+soft-float"
+
+// check -mfloat-abi=hard option for ppc32
+// RUN: %clang -target powerpc-unknown-linux-gnu %s -mfloat-abi=hard -### -o %t.o 2>&1 | FileCheck --check-prefix=CHECK-FLOATABIHARD %s
+// CHECK-FLOATABIHARD-NOT: "-target-feature" "+soft-float"
+
+// check combine -mhard-float -msoft-float option for ppc32
+// RUN: %clang -target powerpc-unknown-linux-gnu %s -mhard-float -msoft-float -### -o %t.o 2>&1 | FileCheck --check-prefix=CHECK-HARDSOFT %s
+// CHECK-HARDSOFT: "-target-feature" "+soft-float"
+
+// check combine -msoft-float -mhard-float option for ppc32
+// RUN: %clang -target powerpc-unknown-linux-gnu %s -msoft-float -mhard-float -### -o %t.o 2>&1 | FileCheck --check-prefix=CHECK-SOFTHARD %s
+// CHECK-SOFTHARD-NOT: "-target-feature" "+soft-float"
+
+// check -mfloat-abi=x option
+// RUN: %clang -target powerpc-unknown-linux-gnu %s -mfloat-abi=x -### -o %t.o 2>&1 | FileCheck --check-prefix=CHECK-ERRMSG %s
+// CHECK-ERRMSG: clang: error: invalid float ABI '-mfloat-abi=x'
+
+// check -msoft-float option for ppc64
+// RUN: %clang -target powerpc64-unknown-linux-gnu %s -msoft-float -### -o %t.o 2>&1 | FileCheck --check-prefix=CHECK-SOFTFLOAT64 %s
+// CHECK-SOFTFLOAT64: clang: error: invalid float ABI 'soft float is not supported for ppc64'
+
+// check -mfloat-abi=soft option for ppc64
+// RUN: %clang -target powerpc64-unknown-linux-gnu %s -mfloat-abi=soft -### -o %t.o 2>&1 | FileCheck --check-prefix=CHECK-FLOATABISOFT64 %s
+// CHECK-FLOATABISOFT64: clang: error: invalid float ABI 'soft float is not supported for ppc64'
+
+// check -msoft-float option for ppc64
+// RUN: %clang -target powerpc64le-unknown-linux-gnu %s -msoft-float -### -o %t.o 2>&1 | FileCheck --check-prefix=CHECK-SOFTFLOAT64le %s
+// CHECK-SOFTFLOAT64le: clang: error: invalid float ABI 'soft float is not supported for ppc64'
+
+// check -mfloat-abi=soft option for ppc64
+// RUN: %clang -target powerpc64le-unknown-linux-gnu %s -mfloat-abi=soft -### -o %t.o 2>&1 | FileCheck --check-prefix=CHECK-FLOATABISOFT64le %s
+// CHECK-FLOATABISOFT64le: clang: error: invalid float ABI 'soft float is not supported for ppc64'
+
 // CHECK: invalid argument '-faltivec' only allowed with 'ppc/ppc64/ppc64le'
 
 // Check that -fno-altivec and -mno-altivec correctly disable the altivec
 // target feature on powerpc.
 
 // RUN: %clang -target powerpc64-unknown-linux-gnu %s -fno-altivec -### -o %t.o 2>&1 | FileCheck --check-prefix=CHECK-1 %s
 // CHECK-1: "-target-feature" "-altivec"