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

Andrea Di Biagio Andrea_DiBiagio at sn.scee.net
Tue Feb 17 12:36:00 PST 2015

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 @@
+  %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?



More information about the llvm-commits mailing list