FPXX modeless calling convention

[jkolek]

Summary

Implemented caller/callee saved registers for FPXX modeless calling convention.

[dsanders]

I have doubts about whether using the odd registers F21 to F31 correctly cause the corresponding double to be saved.

lib/Target/Mips/MipsCallingConv.td

242–247

I'm not sure these are correct. What happens when only F21 is touched by a function? It should cause D10 to be saved/restored. We don't appear to test this case.

getCallPreservedMask() returns CSR_O32_FPXX_NoOdd_RegMask which contains F20, F22, ..., F30 but does not contain D10, D11, ..., D15. I would have expected this function to return the evens from F20 to F30 as well as D10 to D15.

Also, I believe that if you merged these two classes into the same definition like this:

def CSR_O32_FPXX : CalleeSavedRegs<(add (sequence "D%u", 15, 10), RA, FP, (sequence "S%u", 7, 0))> {
  let OtherPreserved = (add F20, F22, F24, F26, F28, F30);
}

then you wouldn't need two definitions and it would make getCallPreservedMask() do the right thing.

lib/Target/Mips/MipsSubtarget.cpp

89

Style nit: This ought to be in the member initializer list

lib/Target/Mips/MipsSubtarget.h

62–64

Would it be easy to make this a member of the MipsABIEnum enum?

As far as I can tell there are only two explicit checks for the O32 ABI in our backend. One is in MipsAsmParser.cpp (search for FeatureO32), the other is in MipsLongBranch.cpp. These would need to become tests for O32 || O32FPXX.

This isn't a blocking issue in my opinion but it would be best to put it in the enum if we can do so easily.

test/CodeGen/Mips/cconv/callee-saved-fpxx.ll

55–61

The O32-FPXX-INV-NOT's in this region are redundant. They are covered on lines 40-43,

[dsanders]

Also, use of FPXX needs to cause a '.module fp=fpxx' to be emitted (you'll need the patch in D4031). That directive can be emitted by a follow-up patch if you prefer. Without it, the binaries will be treated as using the O32 ABI rather than the O32+fpxx ABI.

[jkolek]

lib/Target/Mips/MipsSubtarget.h

62–64

I added this flag according to the way O32 FP32/FP64 are currently implemented. I think that both ways make sense, but if we add O32FPXX to MipsABIEnum, I think that probably O32FP64 should be added there as well. I would like to leave this as it is for now.

[dsanders]

lib/Target/Mips/MipsCallingConv.td

242–247

What happens when only F21 is touched by a function? It should cause D10 to be saved/restored. We don't appear to test this case.

Could you add a test to cover this case? With that and the indent it will LGTM.

243

indent

lib/Target/Mips/MipsSubtarget.h

62–64

That makes sense. We'll leave it as-is for now.

I agree with your comment that O32FP64 belongs in the enum too. We should fix that when we move O32FPXX there. It can be part of general the work to handle ABI's more sensibly.

[jkolek]

Added test case where only F21 is touched by a function, to check that D10 is saved/restored.

[dsanders]

LGTM with one last test addition. Please add a check to callee-saved-fpxx1.ll to check that the base O32 doesn't save 64-bit $f20 when 32-bit $f21 is touched.

[jkolek]

Added check to callee-saved-fpxx1.ll that the base O32 doesn't save 64-bit $f20 when 32-bit $f21 is touched.

[jkolek]

Closed by commit rL212726 (authored by zjovanovic).

lib/Target/Mips/Mips.td

 //===----------------------------------------------------------------------===//
 // Mips Subtarget features                                                    //
 //===----------------------------------------------------------------------===//
 
 def FeatureGP64Bit     : SubtargetFeature<"gp64", "IsGP64bit", "true",
                                 "General Purpose Registers are 64-bit wide.">;
 def FeatureFP64Bit     : SubtargetFeature<"fp64", "IsFP64bit", "true",
                                 "Support 64-bit FP registers.">;
+def FeatureFPXX        : SubtargetFeature<"fpxx", "IsFPXX", "true",
+                                "Support for FPXX.">;
 def FeatureNaN2008     : SubtargetFeature<"nan2008", "IsNaN2008bit", "true",
                                 "IEEE 754-2008 NaN encoding.">;
 def FeatureSingleFloat : SubtargetFeature<"single-float", "IsSingleFloat",
                                 "true", "Only supports single precision float">;
 def FeatureO32         : SubtargetFeature<"o32", "MipsABI", "O32",
                                 "Enable o32 ABI">;
 def FeatureN32         : SubtargetFeature<"n32", "MipsABI", "N32",
                                 "Enable n32 ABI">;

lib/Target/Mips/MipsCallingConv.td

 
 //===----------------------------------------------------------------------===//
 // Callee-saved register lists.
 //===----------------------------------------------------------------------===//
 
 def CSR_SingleFloatOnly : CalleeSavedRegs<(add (sequence "F%u", 31, 20), RA, FP,
                                                (sequence "S%u", 7, 0))>;
 
+def CSR_O32_FPXX : CalleeSavedRegs<(add (sequence "D%u", 15, 10), RA, FP,
+                                        (sequence "S%u", 7, 0))>;
+
+def CSR_O32_FPXX_NoOdd : CalleeSavedRegs<(add F20, F22, F24, F26, F28, F30,
+                                          RA, FP, (sequence "S%u", 7, 0))>;
+
 def CSR_O32 : CalleeSavedRegs<(add (sequence "D%u", 15, 10), RA, FP,
                                    (sequence "S%u", 7, 0))>;
 
 def CSR_O32_FP64 : CalleeSavedRegs<(add (sequence "D%u_64", 31, 20), RA, FP,
                                         (sequence "S%u", 7, 0))>;
 
 def CSR_N32 : CalleeSavedRegs<(add D20_64, D22_64, D24_64, D26_64, D28_64,
                                    D30_64, RA_64, FP_64, GP_64,

lib/Target/Mips/MipsRegisterInfo.cpp

if (Subtarget.isABI_N64())
     return CSR_N64_SaveList;
 
   if (Subtarget.isABI_N32())
     return CSR_N32_SaveList;
 
   if (Subtarget.isFP64bit())
     return CSR_O32_FP64_SaveList;
 
+  if (Subtarget.isFPXX())
+    return CSR_O32_FPXX_SaveList;
+
   return CSR_O32_SaveList;
 }
 
 const uint32_t*
 MipsRegisterInfo::getCallPreservedMask(CallingConv::ID) const {
   if (Subtarget.isSingleFloat())
     return CSR_SingleFloatOnly_RegMask;
 
   if (Subtarget.isABI_N64())
     return CSR_N64_RegMask;
 
   if (Subtarget.isABI_N32())
     return CSR_N32_RegMask;
 
   if (Subtarget.isFP64bit())
     return CSR_O32_FP64_RegMask;
 
+  if (Subtarget.isFPXX())
+    return CSR_O32_FPXX_NoOdd_RegMask;
+
   return CSR_O32_RegMask;
 }
 
 const uint32_t *MipsRegisterInfo::getMips16RetHelperMask() {
   return CSR_Mips16RetHelper_RegMask;
 }
 
 BitVector MipsRegisterInfo::

lib/Target/Mips/MipsSubtarget.h

protected:
   // IsSingleFloat - The target only supports single precision float
   // point operations. This enable the target to use all 32 32-bit
   // floating point registers instead of only using even ones.
   bool IsSingleFloat;
 
   // IsFP64bit - The target processor has 64-bit floating point registers.
   bool IsFP64bit;
 
+  // IsFPXX - MIPS O32 modeless ABI.
+  bool IsFPXX;
+
   // IsNan2008 - IEEE 754-2008 NaN encoding.
   bool IsNaN2008bit;
 
   // IsFP64bit - General-purpose registers are 64 bits wide
   bool IsGP64bit;
 
   // HasVFPU - Processor has a vector floating point unit.
   bool HasVFPU;
bool hasMips32r6() const { return MipsArchVersion == Mips32r6 ||
                                    MipsArchVersion == Mips64r6; }
   bool hasMips64() const { return MipsArchVersion >= Mips64; }
   bool hasMips64r2() const { return MipsArchVersion == Mips64r2; }
   bool hasMips64r6() const { return MipsArchVersion == Mips64r6; }
 
   bool hasCnMips() const { return HasCnMips; }
 
   bool isLittle() const { return IsLittle; }
+  bool isFPXX() const { return IsFPXX; }
   bool isFP64bit() const { return IsFP64bit; }
   bool isNaN2008() const { return IsNaN2008bit; }
   bool isNotFP64bit() const { return !IsFP64bit; }
   bool isGP64bit() const { return IsGP64bit; }
   bool isGP32bit() const { return !IsGP64bit; }
   bool isSingleFloat() const { return IsSingleFloat; }
   bool isNotSingleFloat() const { return !IsSingleFloat; }
   bool hasVFPU() const { return HasVFPU; }

lib/Target/Mips/MipsSubtarget.cpp

: MipsGenSubtargetInfo(TT, CPU, FS), MipsArchVersion(Mips32),
       MipsABI(UnknownABI), IsLittle(little), IsSingleFloat(false),
       IsFP64bit(false), IsNaN2008bit(false), IsGP64bit(false), HasVFPU(false),
       HasCnMips(false), IsLinux(true), HasMips3_32(false), HasMips3_32r2(false),
       HasMips4_32(false), HasMips4_32r2(false), HasMips5_32r2(false),
       InMips16Mode(false), InMips16HardFloat(Mips16HardFloat),
       InMicroMipsMode(false), HasDSP(false), HasDSPR2(false),
       AllowMixed16_32(Mixed16_32 | Mips_Os16), Os16(Mips_Os16), HasMSA(false),
       RM(_RM), OverrideMode(NoOverride), TM(_TM), TargetTriple(TT) {
+  IsFPXX = false;
   std::string CPUName = CPU;
   CPUName = selectMipsCPU(TT, CPUName);
 
   // Parse features string.
   ParseSubtargetFeatures(CPUName, FS);
 
   if (InMips16Mode && !TM->Options.UseSoftFloat) {
     // Hard float for mips16 means essentially to compile as soft float

test/CodeGen/Mips/cconv/callee-saved-fpxx.ll

+; RUN: llc -march=mips -mattr=+fpxx < %s | FileCheck --check-prefix=ALL --check-prefix=O32-FPXX %s
+; RUN: llc -march=mipsel -mattr=+fpxx < %s | FileCheck --check-prefix=ALL --check-prefix=O32-FPXX %s
+; RUN: llc -march=mips -mattr=+fpxx < %s | FileCheck --check-prefix=ALL --check-prefix=O32-FPXX-INV %s
+; RUN: llc -march=mipsel -mattr=+fpxx < %s | FileCheck --check-prefix=ALL --check-prefix=O32-FPXX-INV %s
+
+; RUN-TODO: llc -march=mips64 -mattr=+fpxx < %s | FileCheck --check-prefix=ALL --check-prefix=O32-FPXX %s
+; RUN-TODO: llc -march=mips64el -mattr=+fpxx < %s | FileCheck --check-prefix=ALL --check-prefix=O32-FPXX %s
+; RUN-TODO: llc -march=mips64 -mattr=+fpxx < %s | FileCheck --check-prefix=ALL --check-prefix=O32-FPXX-INV --check-prefix=O32-FPXX-INV %s
+; RUN-TODO: llc -march=mips64el -mattr=+fpxx < %s | FileCheck --check-prefix=ALL --check-prefix=O32-FPXX-INV --check-prefix=O32-FPXX-INV %s
+
+define void @fpu_clobber() nounwind {
+entry:
+    call void asm "# Clobber", "~{$f0},~{$f1},~{$f2},~{$f3},~{$f4},~{$f5},~{$f6},~{$f7},~{$f8},~{$f9},~{$f10},~{$f11},~{$f12},~{$f13},~{$f14},~{$f15},~{$f16},~{$f17},~{$f18},~{$f19},~{$f20},~{$f21},~{$f22},~{$f23},~{$f24},~{$f25},~{$f26},~{$f27},~{$f28},~{$f29},~{$f30},~{$f31}"()
+    ret void
+}
+
+; O32-FPXX-LABEL: fpu_clobber:
+; O32-FPXX-INV-NOT:   sdc1 $f0,
+; O32-FPXX-INV-NOT:   sdc1 $f1,
+; O32-FPXX-INV-NOT:   sdc1 $f2,
+; O32-FPXX-INV-NOT:   sdc1 $f3,
+; O32-FPXX-INV-NOT:   sdc1 $f4,
+; O32-FPXX-INV-NOT:   sdc1 $f5,
+; O32-FPXX-INV-NOT:   sdc1 $f6,
+; O32-FPXX-INV-NOT:   sdc1 $f7,
+; O32-FPXX-INV-NOT:   sdc1 $f8,
+; O32-FPXX-INV-NOT:   sdc1 $f9,
+; O32-FPXX-INV-NOT:   sdc1 $f10,
+; O32-FPXX-INV-NOT:   sdc1 $f11,
+; O32-FPXX-INV-NOT:   sdc1 $f12,
+; O32-FPXX-INV-NOT:   sdc1 $f13,
+; O32-FPXX-INV-NOT:   sdc1 $f14,
+; O32-FPXX-INV-NOT:   sdc1 $f15,
+; O32-FPXX-INV-NOT:   sdc1 $f16,
+; O32-FPXX-INV-NOT:   sdc1 $f17,
+; O32-FPXX-INV-NOT:   sdc1 $f18,
+; O32-FPXX-INV-NOT:   sdc1 $f19,
+; O32-FPXX-INV-NOT:   sdc1 $f21,
+; O32-FPXX-INV-NOT:   sdc1 $f23,
+; O32-FPXX-INV-NOT:   sdc1 $f25,
+; O32-FPXX-INV-NOT:   sdc1 $f27,
+; O32-FPXX-INV-NOT:   sdc1 $f29,
+; O32-FPXX-INV-NOT:   sdc1 $f31,
+
+; O32-FPXX:           addiu $sp, $sp, -48
+; O32-FPXX-DAG:       sdc1 [[F20:\$f20]], [[OFF20:[0-9]+]]($sp)
+; O32-FPXX-DAG:       sdc1 [[F22:\$f22]], [[OFF22:[0-9]+]]($sp)
+; O32-FPXX-DAG:       sdc1 [[F24:\$f24]], [[OFF24:[0-9]+]]($sp)
+; O32-FPXX-DAG:       sdc1 [[F26:\$f26]], [[OFF26:[0-9]+]]($sp)
+; O32-FPXX-DAG:       sdc1 [[F28:\$f28]], [[OFF28:[0-9]+]]($sp)
+; O32-FPXX-DAG:       sdc1 [[F30:\$f30]], [[OFF30:[0-9]+]]($sp)
+; O32-FPXX-DAG:       ldc1 [[F20]], [[OFF20]]($sp)
+; O32-FPXX-DAG:       ldc1 [[F22]], [[OFF22]]($sp)
+; O32-FPXX-DAG:       ldc1 [[F24]], [[OFF24]]($sp)
+; O32-FPXX-INV-NOT:   sdc1 $f25,
+; O32-FPXX-DAG:       ldc1 [[F26]], [[OFF26]]($sp)
+; O32-FPXX-INV-NOT:   sdc1 $f27,
+; O32-FPXX-DAG:       ldc1 [[F28]], [[OFF28]]($sp)
+; O32-FPXX-INV-NOT:   sdc1 $f29,
+; O32-FPXX-DAG:       ldc1 [[F30]], [[OFF30]]($sp)
+; O32-FPXX-INV-NOT:   sdc1 $f31,
+; O32-FPXX:           addiu $sp, $sp, 48