[PATCH] [X86][FastISel] Avoid introducing legacy SSE instructions if the subtarget has AVX.

Michael Kuperstein michael.m.kuperstein at intel.com
Fri Feb 6 13:58:00 PST 2015

Looks good in general, with a couple of comments.

Comment at: lib/Target/X86/X86FastISel.cpp:2019
@@ +2018,3 @@
+      if (HasAVX) {
+        ImplicitDefReg = createResultReg(RC);
+        BuildMI(*FuncInfo.MBB, FuncInfo.InsertPt, DbgLoc,
I have the feeling just reusing OpReg here would be better. 
It's true that having an IMPLICIT_DEF, in theory, gives the regalloc more freedom, but I'm afraid it may just cause false-dependence trouble further on if decides to choose a different register. So it may be better to just force OpReg.

This is what the pattern for the rr version does, btw:

def : Pat<(f64 (fextend FR32:$src)),
    (VCVTSS2SDrr FR32:$src, FR32:$src)>, Requires<[UseAVX]>;

The pattern for the rm version has an IMPLICIT_DEF, but in that case, there is no choice.

Comment at: lib/Target/X86/X86FastISel.cpp:2046
@@ +2045,3 @@
+      // Avoid introducing a legacy SSE instruction if the target has AVX.
+      bool HasAVX = Subtarget->hasAVX();
+      unsigned Opc = HasAVX ? X86::VCVTSD2SSrr : X86::CVTSD2SSrr;
Perhaps this is now large enough to move the common code from the two functions into a helper?

Comment at: test/CodeGen/X86/fast-isel-fptrunc-fpext.ll:25
@@ +24,3 @@
+; ALL-LABEL: single_to_double_rr:
+; SSE: cvtss2sd %xmm0, %xmm0
+; AVX: vcvtss2sd %xmm0, %xmm0, %xmm0
If I remember correctly, this will also match "vcvtss2sd %xmm0, %xmm0, %xmm0" since there's no requirement for the match to start at the beginning of a line, and this is a partial match. Perhaps have an SSE-NOT for the v version?

(Same applies to all testcases)



More information about the llvm-commits mailing list