[mips] Alter register classes for MSA pseudo f16 instructions

[smaksimovic]

Summary

This change introduces additional machine instructions in functions dealing with the expansion of
msa pseudo f16 instructions due to register classes being inappropriate when checked with machine verifier.

[sdardis]

The conversions introduced by this patch are incorrect. fexdo.[hw] and fexupr.[wd] implement the necessary down and up scaling of a floating point value.

The other nit I see if that you're creating virtual registers unconditionally, ideally they should only be created when they are going to be used.

lib/Target/Mips/MipsSEISelLowering.cpp

3596–3604

Rather than unconditionally allocating a virtual GPR64 register, you should instead have the output of COPY_U_H go to Rs, then if !UsingMips32, allocate a GPR64 and perform the SUBREG_TO_REG to writing the temp register, then assign the value of the temp register to Rs.

Then you don't need the conditional on line 3606.

3654–3656

You can sink the creation of the virtual register into the if block, along with "Rt = Tmp;" after emitting the copy.

3742–3745

This is incorrect as it's inconsistent with the logic below.

If the input is a FPR64 on Mips32, this patch changes the expansion to convert a double precision value to a single precision value. This value is then written to all the word lanes.

Lines 3749 - 3763 will write the upper 32bits of the original double into the odd lanes which result in incorrect values in the register when we perform FEXDO_W as that instruction expects doubles in the vector register.

3869–3871

This is incorrect, the input to the conversion is not single precision value, and it's been overwritten by the MTHC1.

[sdardis]

To follow up my previous comments, those conversions should be copies to or from sub-registers.

[smaksimovic]

• Moved virtual registers inside if blocks
• Introduced mtc1/mfc1 counterparts which use FGR64Opnd since copies between FGR32 and FGR64 register classes cannot be performed

[sdardis]

LGTM.

lib/Target/Mips/MipsSEISelLowering.cpp

MipsSETargetLowering::emitST_F16_PSEUDO(MachineInstr &MI,
   //          operand in detail and default to ABI.
   const TargetRegisterClass *RC =
       MI.getOperand(1).isReg() ? RegInfo.getRegClass(MI.getOperand(1).getReg())
                                : (Subtarget.isABI_O32() ? &Mips::GPR32RegClass
                                                         : &Mips::GPR64RegClass);
   const bool UsingMips32 = RC == &Mips::GPR32RegClass;
   unsigned Rs = RegInfo.createVirtualRegister(RC);
 
-  BuildMI(*BB, MI, DL, TII->get(Mips::COPY_U_H), Rs).addReg(Ws).addImm(0);
+  unsigned Tmp = RegInfo.createVirtualRegister(&Mips::GPR32RegClass);
+  unsigned Tmp2 = RegInfo.createVirtualRegister(&Mips::GPR64RegClass);
+  BuildMI(*BB, MI, DL, TII->get(Mips::COPY_U_H), !UsingMips32? Tmp : Rs)
+      .addReg(Ws)
+      .addImm(0);
+  if(!UsingMips32)
+    BuildMI(*BB, MI, DL, TII->get(Mips::SUBREG_TO_REG), Tmp2)
+        .addImm(0)
+        .addReg(Tmp).
+         addImm(Mips::sub_32);
   BuildMI(*BB, MI, DL, TII->get(UsingMips32 ? Mips::SH : Mips::SH64))
-      .addReg(Rs)
+      .addReg(UsingMips32 ? Rs : Tmp2)
       .addReg(Rt)
       .addImm(Imm)
       .addMemOperand(BB->getParent()->getMachineMemOperand(
           &MMO, MMO.getOffset(), MMO.getSize()));
 
   MI.eraseFromParent();
   return BB;
 }
MipsSETargetLowering::emitLD_F16_PSEUDO(MachineInstr &MI,
 
   const bool UsingMips32 = RC == &Mips::GPR32RegClass;
   unsigned Rt = RegInfo.createVirtualRegister(RC);
 
   MachineInstrBuilder MIB =
       BuildMI(*BB, MI, DL, TII->get(UsingMips32 ? Mips::LH : Mips::LH64), Rt);
   for (unsigned i = 1; i < MI.getNumOperands(); i++)
     MIB.add(MI.getOperand(i));
+  unsigned Tmp = RegInfo.createVirtualRegister(&Mips::GPR32RegClass);
+  if(!UsingMips32) 
+    BuildMI(*BB, MI, DL, TII->get(Mips::COPY), Tmp).addReg(Rt, 0, Mips::sub_32);
 
-  BuildMI(*BB, MI, DL, TII->get(Mips::FILL_H), Wd).addReg(Rt);
+  BuildMI(*BB, MI, DL, TII->get(Mips::FILL_H), Wd).addReg(UsingMips32 ? Rt
+                                                                      : Tmp);
 
   MI.eraseFromParent();
   return BB;
 }
 
 // Emit the FPROUND_PSEUDO instruction.
 //
 // Round an FGR64Opnd, FGR32Opnd to an f16.
MipsSETargetLowering::emitFPROUND_PSEUDO(MachineInstr &MI,
                                          bool IsFGR64) const {
 
   // Strictly speaking, we need MIPS32R5 to support MSA. We'll be generous
   // here. It's technically doable to support MIPS32 here, but the ISA forbids
   // it.
   assert(Subtarget.hasMSA() && Subtarget.hasMips32r2());
 
   bool IsFGR64onMips64 = Subtarget.hasMips64() && IsFGR64;
+  bool IsFGR64onMips32 = !Subtarget.hasMips64() && IsFGR64;
 
   const TargetInstrInfo *TII = Subtarget.getInstrInfo();
   DebugLoc DL = MI.getDebugLoc();
   unsigned Wd = MI.getOperand(0).getReg();
   unsigned Fs = MI.getOperand(1).getReg();
 
   MachineRegisterInfo &RegInfo = BB->getParent()->getRegInfo();
   unsigned Wtemp = RegInfo.createVirtualRegister(&Mips::MSA128WRegClass);
   const TargetRegisterClass *GPRRC =
       IsFGR64onMips64 ? &Mips::GPR64RegClass : &Mips::GPR32RegClass;
   unsigned MFC1Opc = IsFGR64onMips64 ? Mips::DMFC1 : Mips::MFC1;
   unsigned FILLOpc = IsFGR64onMips64 ? Mips::FILL_D : Mips::FILL_W;
 
   // Perform the register class copy as mentioned above.
   unsigned Rtemp = RegInfo.createVirtualRegister(GPRRC);
-  BuildMI(*BB, MI, DL, TII->get(MFC1Opc), Rtemp).addReg(Fs);
+  unsigned Rtemp1 = RegInfo.createVirtualRegister(&Mips::FGR32RegClass);  
+  if (IsFGR64onMips32) 
+    BuildMI(*BB,MI,DL,TII->get(Mips::CVT_S_D64),Rtemp1).addReg(Fs);
+  BuildMI(*BB, MI, DL, TII->get(MFC1Opc), Rtemp).addReg(IsFGR64onMips32 ? Rtemp1: Fs);
   BuildMI(*BB, MI, DL, TII->get(FILLOpc), Wtemp).addReg(Rtemp);
   unsigned WPHI = Wtemp;
 
-  if (!Subtarget.hasMips64() && IsFGR64) {
+  if (IsFGR64onMips32) {
     unsigned Rtemp2 = RegInfo.createVirtualRegister(GPRRC);
     BuildMI(*BB, MI, DL, TII->get(Mips::MFHC1_D64), Rtemp2).addReg(Fs);
     unsigned Wtemp2 = RegInfo.createVirtualRegister(&Mips::MSA128WRegClass);
     unsigned Wtemp3 = RegInfo.createVirtualRegister(&Mips::MSA128WRegClass);
     BuildMI(*BB, MI, DL, TII->get(Mips::INSERT_W), Wtemp2)
         .addReg(Wtemp)
         .addReg(Rtemp2)
         .addImm(1);
MipsSETargetLowering::emitFPEXTEND_PSEUDO(MachineInstr &MI,
   if (IsFGR64) {
     WPHI = RegInfo.createVirtualRegister(&Mips::MSA128DRegClass);
     BuildMI(*BB, MI, DL, TII->get(Mips::FEXUPR_D), WPHI).addReg(Wtemp);
   }
 
   // Perform the safety regclass copy mentioned above.
   unsigned Rtemp = RegInfo.createVirtualRegister(GPRRC);
   unsigned FPRPHI = IsFGR64onMips32
-                        ? RegInfo.createVirtualRegister(&Mips::FGR64RegClass)
+                        ? RegInfo.createVirtualRegister(&Mips::FGR32RegClass)
                         : Fd;
   BuildMI(*BB, MI, DL, TII->get(COPYOpc), Rtemp).addReg(WPHI).addImm(0);
   BuildMI(*BB, MI, DL, TII->get(MTC1Opc), FPRPHI).addReg(Rtemp);
 
   if (IsFGR64onMips32) {
     unsigned Rtemp2 = RegInfo.createVirtualRegister(GPRRC);
     BuildMI(*BB, MI, DL, TII->get(Mips::COPY_S_W), Rtemp2)
         .addReg(WPHI)
         .addImm(1);
+    unsigned Rtemp3 = RegInfo.createVirtualRegister(&Mips::FGR64RegClass);
+    BuildMI(*BB,MI,DL,TII->get(Mips::CVT_D64_S),Rtemp3).addReg(FPRPHI);
     BuildMI(*BB, MI, DL, TII->get(Mips::MTHC1_D64), Fd)
-        .addReg(FPRPHI)
+        .addReg(Rtemp3)
         .addReg(Rtemp2);
   }
 
   MI.eraseFromParent();
   return BB;
 }
 
 // Emit the FEXP2_W_1 pseudo instructions.

test/CodeGen/Mips/msa/f16-llvm-ir.ll

 ; RUN: llc -relocation-model=pic -march=mipsel -mcpu=mips32r5 \
-; RUN:     -mattr=+fp64,+msa < %s | FileCheck %s \
+; RUN:     -mattr=+fp64,+msa -verify-machineinstrs < %s | FileCheck %s \
 ; RUN:     --check-prefixes=ALL,MIPS32,MIPSR5,MIPS32-O32,MIPS32R5-O32
 ; RUN: llc -relocation-model=pic -march=mips64el -mcpu=mips64r5 \
-; RUN:     -mattr=+fp64,+msa -target-abi n32 < %s | FileCheck %s \
+; RUN:     -mattr=+fp64,+msa -verify-machineinstrs -target-abi n32 < %s | FileCheck %s \
 ; RUN:     --check-prefixes=ALL,MIPS64,MIPSR5,MIPS64-N32,MIPS64R5-N32
 ; RUN: llc -relocation-model=pic -march=mips64el -mcpu=mips64r5 \
-; RUN:     -mattr=+fp64,+msa -target-abi n64 < %s | FileCheck %s \
+; RUN:     -mattr=+fp64,+msa -verify-machineinstrs -target-abi n64 < %s | FileCheck %s \
 ; RUN:     --check-prefixes=ALL,MIPS64,MIPSR5,MIPS64-N64,MIPS64R5-N64
 
 ; RUN: llc -relocation-model=pic -march=mipsel -mcpu=mips32r6 \
-; RUN:     -mattr=+fp64,+msa < %s | FileCheck %s \
+; RUN:     -mattr=+fp64,+msa -verify-machineinstrs < %s | FileCheck %s \
 ; RUN:     --check-prefixes=ALL,MIPS32,MIPSR6,MIPSR6-O32
 ; RUN: llc -relocation-model=pic -march=mips64el -mcpu=mips64r6 \
-; RUN:     -mattr=+fp64,+msa -target-abi n32 < %s | FileCheck %s \
+; RUN:     -mattr=+fp64,+msa -verify-machineinstrs -target-abi n32 < %s | FileCheck %s \
 ; RUN:     --check-prefixes=ALL,MIPS64,MIPSR6,MIPS64-N32,MIPSR6-N32
 ; RUN: llc -relocation-model=pic -march=mips64el -mcpu=mips64r6 \
-; RUN:     -mattr=+fp64,+msa -target-abi n64 < %s | FileCheck %s \
+; RUN:     -mattr=+fp64,+msa -verify-machineinstrs -target-abi n64 < %s | FileCheck %s \
 ; RUN:     --check-prefixes=ALL,MIPS64,MIPSR6,MIPS64-N64,MIPSR6-N64
 
 
 ; Check the use of frame indexes in the msa pseudo f16 instructions.
 
 @k = external global float
 
 declare float @k2(half *)
; ALL-LABEL: fadd_f64:
   %1 = fpext half %0 to double
 ; ALL:    lh $[[R0:[0-9]+]]
 ; ALL:    fill.h $w[[W0:[0-9]+]], $[[R0]]
 ; ALL:    fexupr.w $w[[W1:[0-9]+]], $w[[W0]]
 ; ALL:    fexupr.d $w[[W2:[0-9]+]], $w[[W1]]
 ; MIPS32: copy_s.w $[[R1:[0-9]+]], $w[[W2]][0]
 ; MIPS32: mtc1 $[[R1]], $f[[F0:[0-9]+]]
 ; MIPS32: copy_s.w $[[R2:[0-9]+]], $w[[W2]][1]
-; MIPS32: mthc1 $[[R2]], $f[[F0]]
+; MIPS32: cvt.d.s $f[[F1:[0-9]+]], $f[[F0]]
+; MIPS32: mthc1 $[[R2]], $f[[F1]]
 ; MIPS64: copy_s.d $[[R2:[0-9]+]], $w[[W2]][0]
 ; MIPS64: dmtc1 $[[R2]], $f[[F0:[0-9]+]]
 
   %2 = load half, half * @h, align 2
   %3 = fpext half %2 to double
   %add = fadd double %1, %3
 
-; ALL: add.d $f[[F1:[0-9]+]], $f[[F0]], $f[[F0]]
+; MIPS32: add.d $f[[F2:[0-9]+]], $f[[F1]], $f[[F1]]
+; MIPS64: add.d $f[[F1:[0-9]+]], $f[[F0]], $f[[F0]]
 
   %4 = fptrunc double %add to half
 
-; MIPS32: mfc1 $[[R2:[0-9]+]], $f[[F1]]
+; MIPS32: cvt.s.d $f[[F3:[0-9]+]], $f[[F2]]
+; MIPS32: mfc1 $[[R2:[0-9]+]], $f[[F3]]
 ; MIPS32: fill.w $w[[W2:[0-9]+]], $[[R2]]
 ; MIPS32: mfhc1 $[[R3:[0-9]+]], $f[[F1]]
 ; MIPS32: insert.w $w[[W2]][1], $[[R3]]
 ; MIPS32: insert.w $w[[W2]][3], $[[R3]]
 
 ; MIPS64: dmfc1 $[[R2:[0-9]+]], $f[[F1]]
 ; MIPS64: fill.d $w[[W2:[0-9]+]], $[[R2]]
 

 ; ALL:          fill.w $w[[W2:[0-9]+]], $[[R2]]
 ; ALL:          fexdo.h $w[[W3:[0-9]+]], $w[[W2]], $w[[W2]]
 ; ALL:          fexupr.w $w[[W4:[0-9]+]], $w[[W3]]
 ; ALL:          fexupr.d $w[[W5:[0-9]+]], $w[[W4]]
 
 ; MIPS32:       copy_s.w $[[R3:[0-9]+]], $w[[W5]][0]
 ; MIPS32:       mtc1 $[[R3]], $f[[F3:[0-9]+]]
 ; MIPS32:       copy_s.w $[[R4:[0-9]+]], $w[[W5]][1]
-; MIPS32:       mthc1 $[[R3]], $f[[F3]]
+; MIPS32:       cvt.d.s $f[[F4:[0-9]+]], $f[[F3]]
+; MIPS32:       mthc1 $[[R3]], $f[[F4]]
 
 ; MIPS64:       copy_s.d $[[R2:[0-9]+]], $w[[W2]][0]
 ; MIPS64:       dmtc1 $[[R2]], $f[[F3:[0-9]+]]
 
-; ALL:          trunc.w.d $f[[F4:[0-9]+]], $f[[F3]]
-; ALL:          mfc1 $[[R4:[0-9]+]], $f[[F4]]
+; MIPS32:       trunc.w.d $f[[F5:[0-9]+]], $f[[F4]]
+; MIPS32:       mfc1 $[[R4:[0-9]+]], $f[[F5]]
+; MIPS64:       trunc.w.d $f[[F4:[0-9]+]], $f[[F3]]
+; MIPS64:       mfc1 $[[R4:[0-9]+]], $f[[F4]]
 ; ALL:          fexupr.d $w[[W6:[0-9]+]], $w[[W1]]
 
 ; MIPS32:       copy_s.w $[[R5:[0-9]+]], $w[[W6]][0]
-; MIPS32:       mtc1 $[[R5]], $f[[F5:[0-9]+]]
+; MIPS32:       mtc1 $[[R5]], $f[[F6:[0-9]+]]
 ; MIPS32:       copy_s.w $[[R6:[0-9]+]], $w[[W6]][1]
-; MIPS32:       mthc1 $[[R6]], $f[[F5]]
+; MIPS32:       cvt.d.s $f[[F7:[0-9]+]], $f[[F6]]
+; MIPS32:       mthc1 $[[R6]], $f[[F7]]
 
 ; MIPS64:       copy_s.d $[[R2:[0-9]+]], $w[[W2]][0]
 ; MIPS64:       dmtc1 $[[R2]], $f[[F5:[0-9]+]]
 
-; ALL:          trunc.w.d $f[[F6:[0-9]]], $f[[F5]]
-; ALL:          mfc1 $[[R7:[0-9]]], $f[[F6]]
+; MIPS32:          trunc.w.d $f[[F8:[0-9]]], $f[[F7]]
+; MIPS32:          mfc1 $[[R7:[0-9]]], $f[[F8]]
+; MIPS64:          trunc.w.d $f[[F6:[0-9]]], $f[[F5]]
+; MIPS64:          mfc1 $[[R7:[0-9]]], $f[[F6]]
 
 ; MIPS32R5-O32: lw $[[R13:[0-9]+]], %got($CPI{{[0-9]+}}_{{[0-9]+}})
 ; MIPS32R5-O32: addiu $[[R14:[0-9]+]], $[[R13]], %lo($CPI{{[0-9]+}}_{{[0-9]+}})
 
 ; MIPS64R5-N32: lw $[[R13:[0-9]+]], %got_page(.LCPI{{[0-9]+}}_{{[0-9]+}})
 ; MIPS64R5-N32: addiu $[[R14:[0-9]+]], $[[R13]], %got_ofst(.LCPI{{[0-9]+}}_{{[0-9]+}})
 
 ; MIPS64R5-N64: ld $[[R13:[0-9]+]], %got_page(.LCPI{{[0-9]+}}_{{[0-9]+}})

 ; ALL:    lh $[[R0:[0-9]+]]
 ; ALL:    fill.h $w[[W0:[0-9]+]], $[[R0]]
 ; ALL:    fexupr.w $w[[W1:[0-9]+]], $w[[W0]]
 ; ALL:    fexupr.d $w[[W2:[0-9]+]], $w[[W1]]
 
 ; MIPS32: copy_s.w $[[R2:[0-9]+]], $w[[W2]][0]
 ; MIPS32: mtc1 $[[R2]], $f[[F0:[0-9]+]]
 ; MIPS32: copy_s.w $[[R3:[0-9]+]], $w[[W2]][1]
-; MIPS32: mthc1 $[[R3]], $f[[F0]]
+; MIPS32: cvt.d.s $f[[F1:[0-9]+]], $f[[F0]]
+; MIPS32: mthc1 $[[R3]], $f[[F1]]
 
 ; MIPS64: copy_s.d $[[R2:[0-9]+]], $w[[W2]][0]
 ; MIPS64: dmtc1 $[[R2]], $f[[F0:[0-9]+]]
 
-; ALL:    trunc.w.d $f[[F1:[0-9]+]], $f[[F0]]
-; ALL:    mfc1 $2, $f[[F1]]
+; MIPS32:    trunc.w.d $f[[F2:[0-9]+]], $f[[F1]]
+; MIPS32:    mfc1 $2, $f[[F2]]
+; MIPS64:    trunc.w.d $f[[F1:[0-9]+]], $f[[F0]]
+; MIPS64:    mfc1 $2, $f[[F1]]
 }
 
 define void @uitofp(i32 %a) {
 entry:
 ; ALL-LABEL: uitofp:
 
 ; MIPS32-O32: ldc1 $f[[F0:[0-9]+]], %lo($CPI{{[0-9]+}}_{{[0-9]+}})
 ; MIPS32-O32: ldc1 $f[[F1:[0-9]+]], 0($sp)
 
 ; MIPS64-N32: ldc1 $f[[F0:[0-9]+]], %got_ofst(.LCPI{{[0-9]+}}_{{[0-9]+}})
 ; MIPS64-N32: ldc1 $f[[F1:[0-9]+]], 8($sp)
 
 ; MIPS64-N64: ldc1 $f[[F0:[0-9]+]], %got_ofst(.LCPI{{[0-9]+}}_{{[0-9]+}})
 ; MIPS64-N64: ldc1 $f[[F1:[0-9]+]], 8($sp)
 
 ; MIPSR5:     sub.d $f[[F2:[0-9]+]], $f[[F1]], $f[[F0]]
 ; MIPSR6-O32: sub.d $f[[F2:[0-9]+]], $f[[F0]], $f[[F1]]
 ; MIPSR6-N32: sub.d $f[[F2:[0-9]+]], $f[[F1]], $f[[F0]]
 ; MIPSR6-N64: sub.d $f[[F2:[0-9]+]], $f[[F1]], $f[[F0]]
 
-; MIPS32:     mfc1 $[[R0:[0-9]+]], $f[[F2]]
+; MIPS32:     cvt.s.d $f[[F3:[0-9]+]], $f[[F2]]
+; MIPS32:     mfc1 $[[R0:[0-9]+]], $f[[F3]]
 ; MIPS32:     fill.w $w[[W0:[0-9]+]], $[[R0]]
 ; MIPS32:     mfhc1 $[[R1:[0-9]+]], $f[[F2]]
 ; MIPS32:     insert.w $w[[W0]][1], $[[R1]]
 ; MIPS32:     insert.w $w[[W0]][3], $[[R1]]
 
 ; MIPS64-N64-DAG: ld $[[R3:[0-9]+]], %got_disp(h)
 ; MIPS64-N32-DAG: lw $[[R3:[0-9]+]], %got_disp(h)
 ; MIPS64-DAG:     dmfc1 $[[R1:[0-9]+]], $f[[F2]]