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

[Bill Schmidt]

Some style issues, and some fragile test-case concerns.


REPOSITORY
  rL LLVM

================
Comment at: lib/Target/PowerPC/PPCFastISel.cpp:962
@@ -961,1 +961,3 @@
+// FIXME: Once fast-isel has better support for VSX, conversions using
+//        direct moves should be implemented
 bool PPCFastISel::SelectIToFP(const Instruction *I, bool IsSigned) {
----------------
Obligatory "add a period" comment.

================
Comment at: lib/Target/PowerPC/PPCFastISel.cpp:1072
@@ -1068,2 +1071,3 @@
+//        direct moves should be implemented
 bool PPCFastISel::SelectFPToI(const Instruction *I, bool IsSigned) {
   MVT DstVT, SrcVT;
----------------
Likewise.

================
Comment at: lib/Target/PowerPC/PPCISelLowering.cpp:5951
@@ -5915,1 +5950,3 @@
                                           SDLoc dl) const {
+  if(Subtarget.hasDirectMove() && Subtarget.isPPC64())
+    return LowerFP_TO_INTDirectMove(Op, DAG, dl);
----------------
Missing space before left parenthesis.

================
Comment at: lib/Target/PowerPC/PPCISelLowering.cpp:6109
@@ +6108,3 @@
+  // However, without FPCVT we can't do most conversions
+  if(Subtarget.hasDirectMove() && Subtarget.isPPC64() && Subtarget.hasFPCVT())
+    return LowerINT_TO_FPDirectMove(Op, DAG, dl);
----------------
Space before parenthesis.

================
Comment at: lib/Target/PowerPC/PPCInstrVSX.td:992
@@ +991,2 @@
+                      [(set f64:$XT, (PPCmtvsrz i32:$rA))]>;
+} // HasDirectMove
----------------
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).

================
Comment at: test/CodeGen/PowerPC/fp-int-conversions-direct-moves.ll:15
@@ +14,3 @@
+; CHECK: mfvsrwz 3, 0
+}
+
----------------
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.

================
Comment at: test/CodeGen/PowerPC/fp-int-conversions-direct-moves.ll:27
@@ +26,3 @@
+; CHECK: mtvsrwz 0, 3
+; CHECK: xscvuxddp 0, 0
+}
----------------
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?)

================
Comment at: test/CodeGen/PowerPC/fp-int-conversions-direct-moves.ll:79
@@ +78,3 @@
+; CHECK: mtvsrwz 0, 3
+; CHECK: xscvuxddp 0, 0
+}
----------------
Same concerns with result reg.

================
Comment at: test/CodeGen/PowerPC/fp-int-conversions-direct-moves.ll:131
@@ +130,3 @@
+; CHECK: mtvsrwa 0, 3
+; CHECK: xscvsxddp 0, 0
+}
----------------
And again.

================
Comment at: test/CodeGen/PowerPC/fp-int-conversions-direct-moves.ll:183
@@ +182,3 @@
+; CHECK: mtvsrwz 0, 3
+; CHECK: xscvuxddp 0, 0
+}
----------------
And again.

================
Comment at: test/CodeGen/PowerPC/fp-int-conversions-direct-moves.ll:235
@@ +234,3 @@
+; CHECK: mtvsrwa 0, 3
+; CHECK: xscvsxddp 0, 0
+}
----------------
And again.

================
Comment at: test/CodeGen/PowerPC/fp-int-conversions-direct-moves.ll:287
@@ +286,3 @@
+; CHECK: mtvsrwz 0, 3
+; CHECK: xscvuxddp 0, 0
+}
----------------
And again.

================
Comment at: test/CodeGen/PowerPC/fp-int-conversions-direct-moves.ll:426
@@ +425,2 @@
+
+!0 = !{!"clang version 3.7.0 (trunk 232940)"}
----------------
Please remove the attributes and metadata.  Delete these trailing lines, and remove the #0 from the function definitions.

http://reviews.llvm.org/D8928

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