[PATCH] D45205: [ARM] FP16 VSEL codegen follow up

[Oliver Stannard via Phabricator via llvm-commits]

olista01 added inline comments.


================
Comment at: lib/Target/ARM/ARMISelLowering.cpp:4529
   if (Subtarget->hasFPARMv8() && !isFloatingPointZero(RHS) &&
-    (TrueVal.getValueType() == MVT::f32 || TrueVal.getValueType() == MVT::f64)) {
+     (TrueVal.getValueType() == MVT::f16 ||
+      TrueVal.getValueType() == MVT::f32 ||
----------------
Does this need to check Subtarget->hasFullFP16()?


================
Comment at: test/CodeGen/ARM/fp16-instructions.ll:769
+; CHECK-LABEL:                 select_cc_ge2:
+; FIXME
+}
----------------
What code actually gets emitted here? If it is correct but sub-optimal we should be testing it, otherwise this patch shouldn't be committed util the generated code is correct.


================
Comment at: test/CodeGen/ARM/fp16-instructions.ll:778
+; CHECK-LABEL:                 select_cc_ge3:
+; CHECK-HARDFP-FULLFP16:       vselge.f16  s0, s{{.}}, s{{.}}
+}
----------------
This only checks the HARDFP-FULLFP16 case, should it also be checking the others? Also, should we check the VCMP instruction, as well as the VSEL?


https://reviews.llvm.org/D45205