[PATCH] [MIPS] OperationAction for FP_TO_FP16, FP16_TO_FP

Pirama Arumuga Nainar pirama at google.com
Mon Apr 20 11:30:48 PDT 2015


================
Comment at: lib/Target/Mips/MipsISelLowering.cpp:226-229
@@ -226,1 +225,6 @@
+  // for f32, f16
+  for (MVT VT : MVT::fp_valuetypes()) {
     setLoadExtAction(ISD::EXTLOAD, VT, MVT::f32, Expand);
+    setLoadExtAction(ISD::EXTLOAD, VT, MVT::f16, Expand);
+  }
+
----------------
dsanders wrote:
> While iterating, this will do 3 strange calls:
>   setLoadExtAction(ISD::EXTLOAD, MVT::f16, MVT::f32, Expand);
>   setLoadExtAction(ISD::EXTLOAD, MVT::f16, MVT::f16, Expand);
>   setLoadExtAction(ISD::EXTLOAD, MVT::f32, MVT::f32, Expand);
> I assume it isn't a problem to do this since EXTLOAD doesn't make sense when the memory type is larger (or the same as) the value type. It might be better to list the valid cases like we do for setTruncStoreAction() below. I don't mind which approach is taken.
fp_valuetypes has larger FP types (f80, f128 etc).  Enumerating all of them will be tedious and  skipping these types is a divergence from current behavior.

The only strange call introduced by this patch is the second one you listed.  Such an EXTLOAD should never be generated, so I'll leave this as is.

================
Comment at: test/CodeGen/Mips/fp16-promote.ll:3-5
@@ +2,5 @@
+
+target datalayout = "E-p:32:32:32-i1:8:8-i8:8:32-i16:16:32-i32:32:32-i64:64:64-f32:32:32-f64:64:64-v64:64:64-n32-S64"
+target triple = "mips--linux-gnu"
+
+; CHECK-LIBCALL-LABEL: test_fadd:
----------------
dsanders wrote:
> Nit: These two lines aren't necessary and it's best not to duplicate the datalayout in the test in case we ever change it. The bit I think might change at some point is the vector part since our vectors only require the elements to be aligned and not the whole vector.
Done

================
Comment at: test/CodeGen/Mips/fp16-promote.ll:64-71
@@ +63,10 @@
+; CHECK-LIBCALL-LABEL: test_vec_fpext_double:
+; CHECK-LIBCALL: %call16(__gnu_h2f_ieee)
+; CHECK-LIBCALL: %call16(__gnu_h2f_ieee)
+; CHECK-LIBCALL: %call16(__gnu_h2f_ieee)
+; CHECK-LIBCALL: cvt.d.s
+; CHECK-LIBCALL: cvt.d.s
+; CHECK-LIBCALL: cvt.d.s
+; CHECK-LIBCALL: %call16(__gnu_h2f_ieee)
+; CHECK-LIBCALL: cvt.d.s
+define <4 x double> @test_vec_fpext_double(<4 x half>* %p) #0 {
----------------
dsanders wrote:
> Just a comment, no change required: This might be a little fragile (the exact order of the output may vary) but I don't see a good way to make it more robust at the moment.
I'll switch to CHECK-DAG for this test.  It's still not robust enough, but is better than the current state.

http://reviews.llvm.org/D8804

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/






More information about the llvm-commits mailing list