[PATCH] Add direct moves to/from VSR and exploit them for FP/INT conversions

[Nemanja Ivanovic]

REPOSITORY
  rL LLVM

================
Comment at: lib/Target/PowerPC/PPCISelLowering.cpp:6066
@@ +6065,3 @@
+  }
+  if (SinglePrec)
+    FP = DAG.getNode(ISD::FP_ROUND, dl, MVT::f32, FP, DAG.getIntPtrConstant(0));
----------------
This is going away in the patch that I'm about to upload. Thanks Bill for catching the register number issue that led to the realization that we have this unnecessary rounding.

================
Comment at: lib/Target/PowerPC/PPCInstrVSX.td:992
@@ +991,2 @@
+                      [(set f64:$XT, (PPCmtvsrz i32:$rA))]>;
+} // HasDirectMove
----------------
wschmidt wrote:
> nemanjai wrote:
> > In the actual commit, this will have both:
> > HasDirectMove, HasVSX
> > 
> > to indicate what the brace terminates.
> In all of the above, please indent the overflow lines so that the first character is underneath the character following the "<" character (as you did for MFVSRWZ, but for none of the others).
Ugh, I don't know how I miss these things - it is so obviously misaligned. Thanks for pointing it out. Fixed and will be part of the next revision.

================
Comment at: test/CodeGen/PowerPC/fp-int-conversions-direct-moves.ll:1-2
@@ +1,3 @@
+; RUN: llc -mcpu=pwr8 -mtriple=powerpc64-unknown-linux-gnu < %s | FileCheck %s
+; RUN: llc -mcpu=pwr8 -mtriple=powerpc64le-unknown-linux-gnu < %s | FileCheck %s
+
----------------
echristo wrote:
> Can probably just use -unknown-unknown here as the OS part of the triple?
Yup, I don't see why not. Will do. Thanks for the comment.

================
Comment at: test/CodeGen/PowerPC/fp-int-conversions-direct-moves.ll:15
@@ +14,3 @@
+; CHECK: mfvsrwz 3, 0
+}
+
----------------
wschmidt wrote:
> Specifying the intermediate register number (0) makes the test case more likely to break in the future.  Please use {{[0-9]+}} so the test isn't reliant on specific register allocation.  The 1 and 3 are ok because they are ABI registers we expect to use.  This applies to all the tests here.
I will turn these into a regular expressions. Thanks for the tip.

================
Comment at: test/CodeGen/PowerPC/fp-int-conversions-direct-moves.ll:27
@@ +26,3 @@
+; CHECK: mtvsrwz 0, 3
+; CHECK: xscvuxddp 0, 0
+}
----------------
wschmidt wrote:
> Here the result register of the xscvuxddp is not an ABI reg so should also use a regexp, not a specific reg.  (Also, why didn't it end up in float reg 1?)
I'll turn these into regular expressions if you would like. To answer the question in parentheses, it does not end up in float reg 1 because it is followed by an frsp since we need to round it to single precision. 

I will change the custom lowering to use PPCISD::FCFID[U]S when converting from any integral value to a single-precision float. I replicated the existing logic which in retrospect does not seem sound (actually I just realized that existing logic only rounds if there is no FPCVT). It uses the SDAG nodes for rounding directly to single-precision only when we are converting i64 to f32 but not for other integral types. Since we are now assuming hasFPCVT for the entire function, there is no harm in refactoring this to skip the need for the extra rounding instruction.

Wow, I am really glad you pointed this out since I didn't really think about why FPR 1 was not used. Thanks.

================
Comment at: test/CodeGen/PowerPC/stfiwx.ll:1-2
@@ -1,3 +1,3 @@
-; RUN: llc < %s -march=ppc32 -mtriple=powerpc-apple-darwin8 -mattr=stfiwx | FileCheck %s
-; RUN: llc < %s -march=ppc32 -mtriple=powerpc-apple-darwin8 -mattr=-stfiwx | FileCheck -check-prefix=CHECK-LS %s
+; RUN: llc < %s -march=ppc32 -mattr=-direct-move -mtriple=powerpc-apple-darwin8 -mattr=stfiwx | FileCheck %s
+; RUN: llc < %s -march=ppc32 -mattr=-direct-move -mtriple=powerpc-apple-darwin8 -mattr=-stfiwx | FileCheck -check-prefix=CHECK-LS %s
 
----------------
echristo wrote:
> How is direct-move getting turned on? I thought it was power8 only?
I was running on a Power 8 system and this failed. However, I believe the assumption of a default CPU may have been removed in a newer revision than the one I was modifying for this patch. I will investigate if this can safely be removed.
I just ran this without the -mattr and it is OK now with a newer revision. Removed the change to this test case.

http://reviews.llvm.org/D8928

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