This is an archive of the discontinued LLVM Phabricator instance.

Differential D4293

FPXX modeless calling convention
ClosedPublic

Authored by jkolek on Jun 25 2014, 8:42 AM.

Details

Reviewers
dsanders
vmedic
Commits
rL212726: [mips] Added FPXX modeless calling convention.
Summary

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

Diff Detail

Repository
rL LLVM

Event Timeline

jkolek updated this revision to Diff 10836.Jun 25 2014, 8:42 AM
jkolek retitled this revision from to FPXX modeless calling convention.
jkolek updated this object.
jkolek edited the test plan for this revision. (Show Details)
jkolek added reviewers: dsanders, vmedic.
jkolek added a subscriber: zoran.jovanovic.
dsanders edited edge metadata.Jun 26 2014, 7:43 AM

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 ↗(On Diff #10836)

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 ↗(On Diff #10836)

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

lib/Target/Mips/MipsSubtarget.h
62–64 ↗(On Diff #10836)

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 ↗(On Diff #10836)

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

dsanders added a comment.Jun 26 2014, 7:50 AM

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 added inline comments.Jun 27 2014, 3:04 AM
lib/Target/Mips/MipsSubtarget.h
62–64 ↗(On Diff #10836)

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.

jkolek updated this revision to Diff 10920.Jun 27 2014, 3:17 AM
jkolek edited edge metadata.
dsanders added inline comments.Jun 27 2014, 3:44 AM
lib/Target/Mips/MipsCallingConv.td
243 ↗(On Diff #10920)

indent

242–247 ↗(On Diff #10836)

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.

lib/Target/Mips/MipsSubtarget.h
62–64 ↗(On Diff #10836)

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 updated this revision to Diff 10927.Jun 27 2014, 5:27 AM
jkolek updated this revision to Diff 10928.Jun 27 2014, 5:49 AM

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

dsanders accepted this revision.Jun 27 2014, 5:52 AM
dsanders edited edge metadata.

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.

This revision is now accepted and ready to land.Jun 27 2014, 5:52 AM
jkolek updated this revision to Diff 10930.Jun 27 2014, 6:35 AM
jkolek edited edge metadata.

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 this revision.Jul 10 2014, 8:44 AM
jkolek updated this revision to Diff 11279.

Closed by commit rL212726 (authored by zjovanovic).

jkolek added a subscriber: Unknown Object (MLST).Nov 18 2014, 5:54 AM

Revision Contents

PathSize
llvm/
trunk/
lib/
Target/
Mips/
2 lines
5 lines
6 lines
4 lines
2 lines
test/
CodeGen/
Mips/
cconv/
58 lines
24 lines

Diff 11279

llvm/trunk/lib/Target/Mips/Mips.td

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

llvm/trunk/lib/Target/Mips/MipsCallingConv.td

Show First 20 Lines • Show All 233 Lines • ▼ Show 20 Lines
//===----------------------------------------------------------------------===// //===----------------------------------------------------------------------===//
// Callee-saved register lists. // Callee-saved register lists.
//===----------------------------------------------------------------------===// //===----------------------------------------------------------------------===//
def CSR_SingleFloatOnly : CalleeSavedRegs<(add (sequence "F%u", 31, 20), RA, FP, def CSR_SingleFloatOnly : CalleeSavedRegs<(add (sequence "F%u", 31, 20), RA, FP,
(sequence "S%u", 7, 0))>; (sequence "S%u", 7, 0))>;
def CSR_O32_FPXX : CalleeSavedRegs<(add (sequence "D%u", 15, 10), RA, FP,
(sequence "S%u", 7, 0))> {
let OtherPreserved = (add (decimate (sequence "F%u", 30, 20), 2));
}
def CSR_O32 : CalleeSavedRegs<(add (sequence "D%u", 15, 10), RA, FP, def CSR_O32 : CalleeSavedRegs<(add (sequence "D%u", 15, 10), RA, FP,
(sequence "S%u", 7, 0))>; (sequence "S%u", 7, 0))>;
def CSR_O32_FP64 : CalleeSavedRegs<(add (sequence "D%u_64", 31, 20), RA, FP, def CSR_O32_FP64 : CalleeSavedRegs<(add (sequence "D%u_64", 31, 20), RA, FP,
(sequence "S%u", 7, 0))>; (sequence "S%u", 7, 0))>;
def CSR_N32 : CalleeSavedRegs<(add D20_64, D22_64, D24_64, D26_64, D28_64, def CSR_N32 : CalleeSavedRegs<(add D20_64, D22_64, D24_64, D26_64, D28_64,
D30_64, RA_64, FP_64, GP_64, D30_64, RA_64, FP_64, GP_64,
Show All 9 Lines

llvm/trunk/lib/Target/Mips/MipsRegisterInfo.cpp

Show First 20 Lines • Show All 87 Lines • ▼ Show 20 Lines if (Subtarget.isABI_N64())
return CSR_N64_SaveList; return CSR_N64_SaveList;
if (Subtarget.isABI_N32()) if (Subtarget.isABI_N32())
return CSR_N32_SaveList; return CSR_N32_SaveList;
if (Subtarget.isFP64bit()) if (Subtarget.isFP64bit())
return CSR_O32_FP64_SaveList; return CSR_O32_FP64_SaveList;
if (Subtarget.isFPXX())
return CSR_O32_FPXX_SaveList;
return CSR_O32_SaveList; return CSR_O32_SaveList;
} }
const uint32_t* const uint32_t*
MipsRegisterInfo::getCallPreservedMask(CallingConv::ID) const { MipsRegisterInfo::getCallPreservedMask(CallingConv::ID) const {
if (Subtarget.isSingleFloat()) if (Subtarget.isSingleFloat())
return CSR_SingleFloatOnly_RegMask; return CSR_SingleFloatOnly_RegMask;
if (Subtarget.isABI_N64()) if (Subtarget.isABI_N64())
return CSR_N64_RegMask; return CSR_N64_RegMask;
if (Subtarget.isABI_N32()) if (Subtarget.isABI_N32())
return CSR_N32_RegMask; return CSR_N32_RegMask;
if (Subtarget.isFP64bit()) if (Subtarget.isFP64bit())
return CSR_O32_FP64_RegMask; return CSR_O32_FP64_RegMask;
if (Subtarget.isFPXX())
return CSR_O32_FPXX_RegMask;
return CSR_O32_RegMask; return CSR_O32_RegMask;
} }
const uint32_t *MipsRegisterInfo::getMips16RetHelperMask() { const uint32_t *MipsRegisterInfo::getMips16RetHelperMask() {
return CSR_Mips16RetHelper_RegMask; return CSR_Mips16RetHelper_RegMask;
} }
BitVector MipsRegisterInfo:: BitVector MipsRegisterInfo::
▲ Show 20 Lines • Show All 137 Lines • Show Last 20 Lines

llvm/trunk/lib/Target/Mips/MipsSubtarget.h

Show First 20 Lines • Show All 56 Lines • ▼ Show 20 Linesprotected:
// IsLittle - The target is Little Endian // IsLittle - The target is Little Endian
bool IsLittle; bool IsLittle;
// IsSingleFloat - The target only supports single precision float // IsSingleFloat - The target only supports single precision float
// point operations. This enable the target to use all 32 32-bit // point operations. This enable the target to use all 32 32-bit
// floating point registers instead of only using even ones. // floating point registers instead of only using even ones.
bool IsSingleFloat; bool IsSingleFloat;
// IsFPXX - MIPS O32 modeless ABI.
bool IsFPXX;
// IsFP64bit - The target processor has 64-bit floating point registers. // IsFP64bit - The target processor has 64-bit floating point registers.
bool IsFP64bit; bool IsFP64bit;
/// Are odd single-precision registers permitted? /// Are odd single-precision registers permitted?
/// This corresponds to -modd-spreg and -mno-odd-spreg /// This corresponds to -modd-spreg and -mno-odd-spreg
bool UseOddSPReg; bool UseOddSPReg;
// IsNan2008 - IEEE 754-2008 NaN encoding. // IsNan2008 - IEEE 754-2008 NaN encoding.
▲ Show 20 Lines • Show All 128 Lines • ▼ Show 20 Linespublic:
bool hasMips64r2() const { bool hasMips64r2() const {
return MipsArchVersion == Mips64r2 || MipsArchVersion == Mips64r6; return MipsArchVersion == Mips64r2 || MipsArchVersion == Mips64r6;
} }
bool hasMips64r6() const { return MipsArchVersion == Mips64r6; } bool hasMips64r6() const { return MipsArchVersion == Mips64r6; }
bool hasCnMips() const { return HasCnMips; } bool hasCnMips() const { return HasCnMips; }
bool isLittle() const { return IsLittle; } bool isLittle() const { return IsLittle; }
bool isFPXX() const { return IsFPXX; }
bool isFP64bit() const { return IsFP64bit; } bool isFP64bit() const { return IsFP64bit; }
bool useOddSPReg() const { return UseOddSPReg; } bool useOddSPReg() const { return UseOddSPReg; }
bool isNaN2008() const { return IsNaN2008bit; } bool isNaN2008() const { return IsNaN2008bit; }
bool isNotFP64bit() const { return !IsFP64bit; } bool isNotFP64bit() const { return !IsFP64bit; }
bool isGP64bit() const { return IsGP64bit; } bool isGP64bit() const { return IsGP64bit; }
bool isGP32bit() const { return !IsGP64bit; } bool isGP32bit() const { return !IsGP64bit; }
bool isSingleFloat() const { return IsSingleFloat; } bool isSingleFloat() const { return IsSingleFloat; }
bool isNotSingleFloat() const { return !IsSingleFloat; } bool isNotSingleFloat() const { return !IsSingleFloat; }
▲ Show 20 Lines • Show All 86 Lines • Show Last 20 Lines

llvm/trunk/lib/Target/Mips/MipsSubtarget.cpp

Show First 20 Lines • Show All 101 Lines • ▼ Show 20 Linesstatic std::string computeDataLayout(const MipsSubtarget &ST) {
return Ret; return Ret;
} }
MipsSubtarget::MipsSubtarget(const std::string &TT, const std::string &CPU, MipsSubtarget::MipsSubtarget(const std::string &TT, const std::string &CPU,
const std::string &FS, bool little, const std::string &FS, bool little,
Reloc::Model _RM, MipsTargetMachine *_TM) Reloc::Model _RM, MipsTargetMachine *_TM)
: MipsGenSubtargetInfo(TT, CPU, FS), MipsArchVersion(Mips32), : MipsGenSubtargetInfo(TT, CPU, FS), MipsArchVersion(Mips32),
MipsABI(UnknownABI), IsLittle(little), IsSingleFloat(false), MipsABI(UnknownABI), IsLittle(little), IsSingleFloat(false),
IsFP64bit(false), UseOddSPReg(true), IsNaN2008bit(false), IsFPXX(false), IsFP64bit(false), UseOddSPReg(true), IsNaN2008bit(false),
IsGP64bit(false), HasVFPU(false), HasCnMips(false), IsLinux(true), IsGP64bit(false), HasVFPU(false), HasCnMips(false), IsLinux(true),
HasMips3_32(false), HasMips3_32r2(false), HasMips4_32(false), HasMips3_32(false), HasMips3_32r2(false), HasMips4_32(false),
HasMips4_32r2(false), HasMips5_32r2(false), InMips16Mode(false), HasMips4_32r2(false), HasMips5_32r2(false), InMips16Mode(false),
InMips16HardFloat(Mips16HardFloat), InMicroMipsMode(false), HasDSP(false), InMips16HardFloat(Mips16HardFloat), InMicroMipsMode(false), HasDSP(false),
HasDSPR2(false), AllowMixed16_32(Mixed16_32 | Mips_Os16), Os16(Mips_Os16), HasDSPR2(false), AllowMixed16_32(Mixed16_32 | Mips_Os16), Os16(Mips_Os16),
HasMSA(false), RM(_RM), OverrideMode(NoOverride), TM(_TM), HasMSA(false), RM(_RM), OverrideMode(NoOverride), TM(_TM),
TargetTriple(TT), TargetTriple(TT),
DL(computeDataLayout(initializeSubtargetDependencies(CPU, FS, TM))), DL(computeDataLayout(initializeSubtargetDependencies(CPU, FS, TM))),
▲ Show 20 Lines • Show All 182 Lines • Show Last 20 Lines

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

; RUN: llc -march=mips -mattr=+o32,+fpxx < %s | FileCheck --check-prefix=ALL --check-prefix=O32-FPXX %s
; RUN: llc -march=mipsel -mattr=+o32,+fpxx < %s | FileCheck --check-prefix=ALL --check-prefix=O32-FPXX %s
; RUN: llc -march=mips -mattr=+o32,+fpxx < %s | FileCheck --check-prefix=ALL --check-prefix=O32-FPXX-INV %s
; RUN: llc -march=mipsel -mattr=+o32,+fpxx < %s | FileCheck --check-prefix=ALL --check-prefix=O32-FPXX-INV %s
; RUN-TODO: llc -march=mips64 -mattr=+o32,+fpxx < %s | FileCheck --check-prefix=ALL --check-prefix=O32-FPXX %s
; RUN-TODO: llc -march=mips64el -mattr=+o32,+fpxx < %s | FileCheck --check-prefix=ALL --check-prefix=O32-FPXX %s
; RUN-TODO: llc -march=mips64 -mattr=+o32,+fpxx < %s | FileCheck --check-prefix=ALL --check-prefix=O32-FPXX-INV --check-prefix=O32-FPXX-INV %s
; RUN-TODO: llc -march=mips64el -mattr=+o32,+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-DAG: ldc1 [[F26]], [[OFF26]]($sp)
; O32-FPXX-DAG: ldc1 [[F28]], [[OFF28]]($sp)
; O32-FPXX-DAG: ldc1 [[F30]], [[OFF30]]($sp)
; O32-FPXX: addiu $sp, $sp, 48

llvm/trunk/test/CodeGen/Mips/cconv/callee-saved-fpxx1.ll

; RUN: llc -march=mips -mattr=+o32,+fp64 < %s | FileCheck --check-prefix=O32-FP64-INV %s
; RUN: llc -march=mipsel -mattr=+o32,+fp64 < %s | FileCheck --check-prefix=O32-FP64-INV %s
; RUN: llc -march=mips -mattr=+o32,+fpxx < %s | FileCheck --check-prefix=O32-FPXX %s
; RUN: llc -march=mipsel -mattr=+o32,+fpxx < %s | FileCheck --check-prefix=O32-FPXX %s
; RUN-TODO: llc -march=mips64 -mattr=+o32,+fpxx < %s | FileCheck --check-prefix=O32-FPXX %s
; RUN-TODO: llc -march=mips64el -mattr=+o32,+fpxx < %s | FileCheck --check-prefix=O32-FPXX %s
define void @fpu_clobber() nounwind {
entry:
call void asm "# Clobber", "~{$f21}"()
ret void
}
; O32-FPXX-LABEL: fpu_clobber:
; O32-FPXX: addiu $sp, $sp, -8
; O32-FP64-INV-NOT: sdc1 $f20,
; O32-FPXX-DAG: sdc1 [[F20:\$f20]], [[OFF20:[0-9]+]]($sp)
; O32-FPXX-DAG: ldc1 [[F20]], [[OFF20]]($sp)
; O32-FPXX: addiu $sp, $sp, 8