[PATCH] Implement floating point compare for mips fast-isel

Daniel Sanders daniel.sanders at imgtec.com
Wed Oct 8 06:45:21 PDT 2014


LGTM with one new formatting nit.

================
Comment at: lib/Target/Mips/MipsFastISel.cpp:625
@@ +624,3 @@
+    case CmpInst::FCMP_OEQ:
+      Opc = IsFloat ? Mips::C_EQ_S : Mips::C_EQ_D32;
+      CondMovOpc = Mips::MOVT_I;
----------------
dsanders wrote:
> C_EQ_D32 needs to be C_EQ_D64 for 64-bit FPU's and we don't guard against this.
> 
> Similarly for the other compares.
Done

================
Comment at: lib/Target/Mips/MipsFastISel.cpp:640-647
@@ +639,10 @@
+      break;
+    case CmpInst::FCMP_OGT:
+      Opc = IsFloat ? Mips::C_OLE_S : Mips::C_OLE_D32;
+      CondMovOpc = Mips::MOVF_I;
+      break;
+    case CmpInst::FCMP_OGE:
+      Opc = IsFloat ? Mips::C_OLT_S : Mips::C_OLT_D32;
+      CondMovOpc = Mips::MOVF_I;
+      break;
+    default:
----------------
dsanders wrote:
> dsanders wrote:
> > These two aren't correct when one of the operands is NaN. 'NaN ogt 0.0' is false because they are unordered, but '!(NaN ole 0.0)' is true. Swap the LHS/RHS instead.
> After chatting with Reed, I've taken a look at the SelectionDAG implementation and found that it is using c.ule.[ds] and c.ule.[ds] for FCMP_OGT and FCMP_OGE respectively and then inverting the boolean result before the value is consumed. This is valid too and we should probably go with that for the sake of consistency.
Done

================
Comment at: test/CodeGen/Mips/Fast-ISel/fpcmpa.ll:19-26
@@ +18,10 @@
+; CHECK-LABEL: feq1:
+; CHECK-DAG:    lw	$[[REG_F2_GOT:[0-9]+]], %got(f2)(${{[0-9]+}})
+; CHECK-DAG:    lw	$[[REG_F1_GOT:[0-9]+]], %got(f1)(${{[0-9]+}})
+; CHECK-DAG:	lwc1	$f[[REG_F2:[0-9]+]], 0($[[REG_F2_GOT]])
+; CHECK-DAG:	lwc1	$f[[REG_F1:[0-9]+]], 0($[[REG_F1_GOT]])
+; CHECK-DAG:	addiu	$[[REG_ZERO:[0-9]+]], $zero, 0
+; CHECK-DAG:	addiu	$[[REG_ONE:[0-9]+]], $zero, 1
+; CHECK:  c.eq.s  $f[[REG_F1]], $f[[REG_F2]]
+; CHECK:  movt  $[[REG_ZERO]], $[[REG_ONE]], $fcc0
+
----------------
Formatting nit: Could you line up the start of each instruction?

Likewise below

================
Comment at: test/CodeGen/Mips/Fast-ISel/fpcmpa.ll:20
@@ +19,3 @@
+; CHECK:  c.eq.s  $f{{[0-9]+}}, $f{{[0-9]+}}
+; CHECK:  movt  ${{[0-9]+}}, ${{[0-9]+}}, $fcc0
+
----------------
dsanders wrote:
> We ought to check that the registers are in the right order.
> 
> Likewise for the tests below.
Done

http://reviews.llvm.org/D5567






More information about the llvm-commits mailing list