[PowerPC] Fix issue with inline asm - soft float mode

[spetrovic]

Summary

This patch prevents floating point register constraints in soft float mode.

[jhibbits]

I really don't like this. It should be possible to mix soft-float, Altivec, VSX, and SPE in the same file using inline asm. I put a check in for SPE codegen specifically to permit inline asm for other floating point models.

What is the motivation for adding this?

[jhibbits]

I'd like to amend my previous comment: FreeBSD, and I'd guess Linux, too, explicitly builds the kernel with -msoft-float, in order to prevent FPU-consuming optimizations. However, we still need to be able to save and restore FPU context, which requires inline asm (or a completely asm file, not something we want). This change would prevent us being able to do that.

[spetrovic]

For this example:

int foo ()
{

double x,y;
__asm__ ("fctiw %0,%1" : "=f"(x) : "f"(y));
return x;

}

clang throws an assert currently for ppc32 soft float. Gcc reports an error similar as in this patch, as well as clang for arm. If I'm wrong with this approach, could you suggest some other solution.

[jhibbits]

Reading through the diff again, closer, and checking the FreeBSD source, this is acceptable. FreeBSD only uses the base register, and hard-codes register numbers, so doesn't go through float register allocation.

[jhibbits]

Can you adjust the summary? This doesn't prevent floating point instructions completely, it only prevents floating point register constraints. The assertion that it prevents all floating point instructions is what I first took issue with, so please make it explicit that this is *only* regarding register constraints.

[spetrovic]

Sure, I will change summary, thanks for review!

lib/Basic/Targets/PPC.h

class LLVM_LIBRARY_VISIBILITY PPCTargetInfo : public TargetInfo {
   } ArchDefineTypes;
 
 
   ArchDefineTypes ArchDefs = ArchDefineNone;
   static const Builtin::Info BuiltinInfo[];
   static const char *const GCCRegNames[];
   static const TargetInfo::GCCRegAlias GCCRegAliases[];
   std::string CPU;
+  enum PPCFloatABI { HardFloat, SoftFloat } FloatABI;
 
   // Target cpu features.
   bool HasAltivec = false;
   bool HasVSX = false;
   bool HasP8Vector = false;
   bool HasP8Crypto = false;
   bool HasDirectMove = false;
   bool HasQPX = false;
public:
 
   bool validateAsmConstraint(const char *&Name,
                              TargetInfo::ConstraintInfo &Info) const override {
     switch (*Name) {
     default:
       return false;
     case 'O': // Zero
       break;
-    case 'b': // Base register
     case 'f': // Floating point register
+      // Don't use floating point registers on soft float ABI.
+      if (FloatABI == SoftFloat)
+        return false;
+    case 'b': // Base register
       Info.setAllowsRegister();
       break;
     // FIXME: The following are added to allow parsing.
     // I just took a guess at what the actions should be.
     // Also, is more specific checking needed?  I.e. specific registers?
     case 'd': // Floating point register (containing 64-bit value)
     case 'v': // Altivec vector register
+      // Don't use floating point and altivec vector registers
+      // on soft float ABI
+      if (FloatABI == SoftFloat)
+        return false;
       Info.setAllowsRegister();
       break;
     case 'w':
       switch (Name[1]) {
       case 'd': // VSX vector register to hold vector double data
       case 'f': // VSX vector register to hold vector float data
       case 's': // VSX vector register to hold scalar float data
       case 'a': // Any VSX register

lib/Basic/Targets/PPC.cpp

#define LIBBUILTIN(ID, TYPE, ATTRS, HEADER)                                    \
   {#ID, TYPE, ATTRS, HEADER, ALL_LANGUAGES, nullptr},
 #include "clang/Basic/BuiltinsPPC.def"
 };
 
 /// handleTargetFeatures - Perform initialization based on the user
 /// configured set of features.
 bool PPCTargetInfo::handleTargetFeatures(std::vector<std::string> &Features,
                                          DiagnosticsEngine &Diags) {
+  FloatABI = HardFloat;
   for (const auto &Feature : Features) {
     if (Feature == "+altivec") {
       HasAltivec = true;
     } else if (Feature == "+vsx") {
       HasVSX = true;
     } else if (Feature == "+bpermd") {
       HasBPERMD = true;
     } else if (Feature == "+extdiv") {
       HasExtDiv = true;
     } else if (Feature == "+power8-vector") {
       HasP8Vector = true;
     } else if (Feature == "+crypto") {
       HasP8Crypto = true;
     } else if (Feature == "+direct-move") {
       HasDirectMove = true;
     } else if (Feature == "+qpx") {
       HasQPX = true;
     } else if (Feature == "+htm") {
       HasHTM = true;
     } else if (Feature == "+float128") {
       HasFloat128 = true;
     } else if (Feature == "+power9-vector") {
       HasP9Vector = true;
+    } else if (Feature == "-hard-float") {
+      FloatABI = SoftFloat;
     }
     // TODO: Finish this list and add an assert that we've handled them
     // all.
   }
 
   return true;
 }
 

test/Driver/ppc-inlineasm-sf.c

+// RUN: not %clang -target powerpc-unknown-linux -O2 -fPIC -m32 -msoft-float %s -o %t.o 2>&1 | FileCheck --check-prefix=CHECK-ERRMSG %s
+int foo ()
+{
+  double x,y;
+  int a;
+  __asm__ ("fctiw %0,%1" : "=f"(x) : "f"(y));
+  // CHECK-ERRMSG:      error: invalid output constraint '=f' in asm
+  // CHECK-ERRMSG-NEXT:  __asm__ ("fctiw %0,%1" : "=f"(x) : "f"(y));
+  __asm__ ("fctiw %0,%1" : "=d"(x) : "d"(y));
+  // CHECK-ERRMSG: error: invalid output constraint '=d' in asm
+  // CHECK-ERRMSG-NEXT: __asm__ ("fctiw %0,%1" : "=d"(x) : "d"(y));
+  __asm__ ("vec_dss %0" : "=v"(a));
+  // CHECK-ERRMSG: error: invalid output constraint '=v' in asm
+  // CHECK-ERRMSG-NEXT: __asm__ ("vec_dss %0" : "=v"(a));
+}
+