[PATCH] [X86][FastISel] Teach how to select float-half conversion intrinsics.

[Andrea Di Biagio]

Hi Quentin,


================
Comment at: lib/Target/X86/X86FastISel.cpp:2149
@@ +2148,3 @@
+    if (IsFloatToHalf && !Op->getType()->isFloatTy())
+      return false;
+
----------------
qcolombet wrote:
> Shouldn’t we have some checks that the type is not double for any cases?
Right, I should check that neither the operand nor the return type is double.
I didn't take into account the fact that the intrinsic allows any floating point type.

================
Comment at: lib/Target/X86/X86FastISel.cpp:2162
@@ +2161,3 @@
+    unsigned ResultReg = createResultReg(RC);
+    BuildMI(*FuncInfo.MBB, FuncInfo.InsertPt, DbgLoc,
+            TII.get(TargetOpcode::COPY), ResultReg).addReg(InputReg);
----------------
qcolombet wrote:
> I think it would be cleaner to generate:
> res = implicit_def
> res2 = insert_subreg res, inputreg, 0
> 
> A copy with mismatching size sounds wrong to me.
Ok, I will change it.

================
Comment at: lib/Target/X86/X86FastISel.cpp:2182
@@ +2181,3 @@
+    MIB = BuildMI(*FuncInfo.MBB, FuncInfo.InsertPt, DbgLoc,
+                  TII.get(TargetOpcode::COPY), ResultReg);
+    MIB.addReg(InputReg, RegState::Kill);
----------------
qcolombet wrote:
> EXTRACT_SUBREG here I believe.
I will fix it.

================
Comment at: test/CodeGen/X86/fast-isel-float-half-convertion.ll:7
@@ +6,3 @@
+entry:
+  %0 = call i16 @llvm.convert.to.fp16.f32(float %a)
+  ret i16 %0
----------------
qcolombet wrote:
> Could you add tests with doubles?
> 
> I may be wrong but I thought the intrinsic allows any floating type.
Right, the intrinsic allows any floating point type.
What if I add those tests into a separate test file maybe an XFAIL test)?

My concern is that if I add extra tests for doubles in this same file, then the test will start failing because of flag -fast-isel-abort. What do you think?

http://reviews.llvm.org/D7673

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