[wip][mips] Use AltOrders to prevent using odd FP-registers

[atanasyan]

Summary

To disable using of odd floating-point registers (O32 ABI and -mno-odd-spreg command line option) such registers and their super-registers added to the set of reserved registers. In general, it works. But there is at least one problem - in case of enabled machine verifier pass some floating-point tests failed because live ranges of register units that are reserved is not empty and verification pass failed with "Live segment doesn't end at a valid instruction" error message.

There is D35985 patch which tries to solve the problem by explicit removing of register units. This solution did not get approval.

I would like to use another approach for prevent using odd floating-point registers - define AltOrders and AltOrderSelect for MIPS floating point register classes. Such AltOrders contains reduced set of registers. At first glance, such solution does not break any test cases and allows enabling machine instruction verification for all MIPS test cases.

Does anybody see any problems on this way?

[Petar.Avramovic]

I don't see other reason for registers form OddSPRegClass to be declared as as Reserved other then to affect Allocation order in register allocation.
From description of nooddspreg : "Disable odd numbered single-precision registers". I would say that we only need to make sure that odd float registers can't be used during register allocation that is to affect Allocation order in some way to make sure odd registers don't appear there. I saw that similar problem for msa was handled with use of different register class (MSA128WRegClass or MSA128WEvensRegClass) in MipsSETargetLowering::emitCOPY_FW.
Here this is done with AltOrders without having to define registers from OddSPRegClass as Reserved. MipsGenRegisterInfo.inc also got smaller, everything looks good from my side.

What I don't really understand purpose of the affected test, it forces use of $f13 with asm instruction despite the fact it is disabled with +nooddspreg option. Shouldn't we just report an error that odd register can't be used with +nooddspreg?

[sdardis]

My initial comments are that this looks ok, but I would like a longer look at it.

llvm/lib/Target/Mips/MipsRegisterInfo.td

385 ↗

(On Diff #192206)

Nit: Can you provide a comment summarising the reason for the AltOrder additions?

394 ↗

(On Diff #192206)

To the best of my knowledge, the FGRH32 regclass is unused. I did remove it locally and run the test-suite, and as best I can recall it made no difference but I didn't run all configurations.

Dealing with that would be a separate commit.

414 ↗

(On Diff #192206)

Nit: Can you provide a comment summarising the reason for the AltOrder additions?

[atanasyan]

• Use more compact definitions of AltOrders
• Add comments

• Thanks for pointing to the MSA128WEvensRegClass. I'm going to investigate a possibility to do the same trick with AltOrders in this case. If it's impossible, find the reason.
• Both GNU assembler and LLVM show warning if odd floating-point register appears in an asm file. As far as I understand the -mno-odd-spreg prevents using odd registers by backend, but does not prevent inserting such registers into asm file directly by a user.
• Agreed, FGRH32 looks redundant.

[Petar.Avramovic]

Both GNU assembler and LLVM show warning if odd floating-point register appears in an asm file. As far as I understand the -mno-odd-spreg prevents using odd registers by backend, but does not prevent inserting such registers into asm file directly by a user.

Thanks for clarification.

llvm/lib/Target/Mips/MipsRegisterInfo.td

387 ↗

(On Diff #192526)

let AltOrders = [(decimate FGR32, 2)]; also works here.

[sdardis]

I think this is a viable approach for this issue. Based on the dumps from -debug I can see that the backend should only be selecting even register for FGR32.

I'd suggest dropping the WIP tag and waiting a day or two to see if @dsanders / @Petar.Avramovic / llvm-commits has any other suggestions.

LGTM.

[atanasyan]

Thanks for review.

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

const RegisterBank &MipsRegisterBankInfo::getRegBankFromRegClass(
   case Mips::CPU16Regs_and_GPRMM16MovePPairSecondRegClassID:
   case Mips::GPRMM16MoveP_and_CPU16Regs_and_GPRMM16ZeroRegClassID:
   case Mips::GPRMM16MovePPairFirst_and_GPRMM16MovePPairSecondRegClassID:
   case Mips::SP32RegClassID:
     return getRegBank(Mips::GPRBRegBankID);
   case Mips::FGRCCRegClassID:
   case Mips::FGR64RegClassID:
   case Mips::AFGR64RegClassID:
-  case Mips::AFGR64_and_OddSPRegClassID:
     return getRegBank(Mips::FPRBRegBankID);
   default:
     llvm_unreachable("Register class not supported");
   }
 }
 
 const RegisterBankInfo::InstructionMapping &
 MipsRegisterBankInfo::getInstrMapping(const MachineInstr &MI) const {

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

getReservedRegs(const MachineFunction &MF) const {
   }
 
   // Reserve GP if small section is used.
   if (Subtarget.useSmallSection()) {
     Reserved.set(Mips::GP);
     Reserved.set(Mips::GP_64);
   }
 
-  if (Subtarget.isABI_O32() && !Subtarget.useOddSPReg()) {
-    for (const auto &Reg : Mips::OddSPRegClass)
-      Reserved.set(Reg);
-  }
-
   return Reserved;
 }
 
 bool
 MipsRegisterInfo::requiresRegisterScavenging(const MachineFunction &MF) const {
   return true;
 }
 

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

 
 // 64bit fp:
 // * FGR64  - 32 64-bit registers
 // * AFGR64 - 16 32-bit even registers (32-bit FP Mode)
 //
 // 32bit fp:
 // * FGR32 - 16 32-bit even registers
 // * FGR32 - 32 32-bit registers (single float only mode)
-def FGR32 : RegisterClass<"Mips", [f32], 32, (sequence "F%u", 0, 31)>;
+def FGR32 : RegisterClass<"Mips", [f32], 32, (sequence "F%u", 0, 31)> {
+  // Do not allocate odd registers when given -mattr=+nooddspreg.
+  let AltOrders = [(decimate FGR32, 2)];
+  let AltOrderSelect = [{
+    const auto & S = MF.getSubtarget<MipsSubtarget>();
+    return S.isABI_O32() && !S.useOddSPReg();
+  }];
+}
 
 def FGRH32 : RegisterClass<"Mips", [f32], 32, (sequence "F_HI%u", 0, 31)>,
-             Unallocatable;
+             Unallocatable {
+  // Do not allocate odd registers when given -mattr=+nooddspreg.
+  let AltOrders = [(decimate FGRH32, 2)];
+  let AltOrderSelect = [{
+    const auto & S = MF.getSubtarget<MipsSubtarget>();
+    return S.isABI_O32() && !S.useOddSPReg();
+  }];
+}
 
 def AFGR64 : RegisterClass<"Mips", [f64], 64, (add
   // Return Values and Arguments
   D0, D1,
   // Not preserved across procedure calls
   D2, D3, D4, D5,
   // Return Values and Arguments
   D6, D7,
   // Not preserved across procedure calls
   D8, D9,
   // Callee save
   D10, D11, D12, D13, D14, D15)>;
 
-def FGR64 : RegisterClass<"Mips", [f64], 64, (sequence "D%u_64", 0, 31)>;
-
-// Used to reserve odd registers when given -mattr=+nooddspreg
-// FIXME: Remove double precision registers from this set.
-def OddSP : RegisterClass<"Mips", [f32], 32,
-                          (add (decimate (sequence "F%u", 1, 31), 2),
-                               (decimate (sequence "F_HI%u", 1, 31), 2),
-                               (decimate (sequence "D%u", 1, 15), 2),
-                               (decimate (sequence "D%u_64", 1, 31), 2))>,
-            Unallocatable;
+def FGR64 : RegisterClass<"Mips", [f64], 64, (sequence "D%u_64", 0, 31)> {
+  // Do not allocate odd registers when given -mattr=+nooddspreg.
+  let AltOrders = [(decimate FGR64, 2)];
+  let AltOrderSelect = [{
+    const auto & S = MF.getSubtarget<MipsSubtarget>();
+    return S.isABI_O32() && !S.useOddSPReg();
+  }];
+}
 
 // FP control registers.
 def CCR : RegisterClass<"Mips", [i32], 32, (sequence "FCR%u", 0, 31)>,
           Unallocatable;
 
 // FP condition code registers.
 def FCC : RegisterClass<"Mips", [i32], 32, (sequence "FCC%u", 0, 7)>,
           Unallocatable;

llvm/trunk/test/CodeGen/Mips/no-odd-spreg-msa.ll

 ; RUN: llc -march=mipsel -mcpu=mips32r5 -mattr=+fp64,+msa,-nooddspreg \
-; RUN:   -no-integrated-as -relocation-model=pic < %s | \
+; RUN:   -verify-machineinstrs -no-integrated-as -relocation-model=pic < %s | \
 ; RUN:   FileCheck %s -check-prefixes=ALL,ODDSPREG
 ; RUN: llc -march=mipsel -mcpu=mips32r5 -mattr=+fp64,+msa,+nooddspreg \
-; RUN:   -no-integrated-as -relocation-model=pic < %s | \
+; RUN:   -verify-machineinstrs -no-integrated-as -relocation-model=pic < %s | \
 ; RUN:   FileCheck %s -check-prefixes=ALL,NOODDSPREG
 
 @v4f32 = global <4 x float> zeroinitializer
 
 define void @msa_insert_0(float %a) {
 entry:
   ; Force the float into an odd-numbered register using named registers and
   ; load the vector.
entry:
   call void asm sideeffect "teqi $$zero, 1", "~{$f0},~{$f1},~{$f2},~{$f3},~{$f4},~{$f5},~{$f6},~{$f7},~{$f8},~{$f9},~{$f10},~{$f11},~{$f14},~{$f15},~{$f16},~{$f17},~{$f18},~{$f19},~{$f20},~{$f21},~{$f22},~{$f23},~{$f24},~{$f25},~{$f26},~{$f27},~{$f28},~{$f29},~{$f30},~{$f31}"()
   %1 = insertelement <4 x float> %0, float %b, i32 0
   store <4 x float> %1, <4 x float>* @v4f32
   ret void
 }
 
 ; ALL-LABEL:  msa_insert_0:
 ; ALL:            mov.s $f13, $f12
+; NOODDSPREG:     mov.s $f[[F0:[0-9]+]], $f13
 ; ALL:            lw $[[R0:[0-9]+]], %got(v4f32)(
 ; ALL:            ld.w $w[[W0:[0-9]+]], 0($[[R0]])
-; NOODDSPREG:     mov.s $f[[F0:[0-9]+]], $f13
 ; NOODDSPREG:     insve.w $w[[W0]][0], $w[[F0]][0]
 ; ODDSPREG:       insve.w $w[[W0]][0], $w13[0]
 ; ALL:            teqi $zero, 1
 ; ALL-NOT: sdc1
 ; ALL-NOT: ldc1
 ; ALL:            st.w $w[[W0]], 0($[[R0]])
 
 define void @msa_insert_1(float %a) {
entry:
   call void asm sideeffect "teqi $$zero, 1", "~{$f0},~{$f1},~{$f2},~{$f3},~{$f4},~{$f5},~{$f6},~{$f7},~{$f8},~{$f9},~{$f10},~{$f11},~{$f14},~{$f15},~{$f16},~{$f17},~{$f18},~{$f19},~{$f20},~{$f21},~{$f22},~{$f23},~{$f24},~{$f25},~{$f26},~{$f27},~{$f28},~{$f29},~{$f30},~{$f31}"()
   %1 = insertelement <4 x float> %0, float %b, i32 1
   store <4 x float> %1, <4 x float>* @v4f32
   ret void
 }
 
 ; ALL-LABEL:  msa_insert_1:
 ; ALL:            mov.s $f13, $f12
+; NOODDSPREG:     mov.s $f[[F0:[0-9]+]], $f13
 ; ALL:            lw $[[R0:[0-9]+]], %got(v4f32)(
 ; ALL:            ld.w $w[[W0:[0-9]+]], 0($[[R0]])
-; NOODDSPREG:     mov.s $f[[F0:[0-9]+]], $f13
 ; NOODDSPREG:     insve.w $w[[W0]][1], $w[[F0]][0]
 ; ODDSPREG:       insve.w $w[[W0]][1], $w13[0]
 ; ALL:            teqi $zero, 1
 ; ALL-NOT: sdc1
 ; ALL-NOT: ldc1
 ; ALL:            st.w $w[[W0]], 0($[[R0]])
 
 define float @msa_extract_0() {