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

[Andrea Di Biagio]

Hi Michael,

thanks for the review. I will send a new version of the patch that addresses all your comments.

-Andrea


================
Comment at: lib/Target/X86/X86FastISel.cpp:2019
@@ +2018,3 @@
+      if (HasAVX) {
+        ImplicitDefReg = createResultReg(RC);
+        BuildMI(*FuncInfo.MBB, FuncInfo.InsertPt, DbgLoc,
----------------
mkuper wrote:
> 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.
Right, I will remove the IMPLICIT_DEF and just use OpReg for both operands in the AVX case.
The reason why I ended up using IMPLICIT_DEF was to give a chance to FastISel to also match the 'register-memory' variants of scalar float/double convert.

================
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;
----------------
mkuper wrote:
> Perhaps this is now large enough to move the common code from the two functions into a helper?
Good point. I'll try to move the common code into a helper function.

================
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
----------------
mkuper wrote:
> 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)
Sure, I will change the tests.

http://reviews.llvm.org/D7438

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