[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