[PATCH] D20239: [Power9] Add codegen for VSX word insert/extract instructions

[Kit Barton via llvm-commits]

kbarton accepted this revision.
kbarton added a comment.
This revision is now accepted and ready to land.

Aside from minor comments, this LGTM.


================
Comment at: lib/Target/PowerPC/PPCISelLowering.cpp:1500-1508
@@ -1493,1 +1499,11 @@
 
+/// isXXINSERTWMask - Return true if this VECTOR_SHUFFLE can be handled by
+/// the XXINSERTW instruction introduced in ISA 3.0. This is essentially any
+/// shuffle of v4f32/v4i32 vectors that just inserts one element from one vector
+/// into the other. This function will also set a couple of
+/// output parameters for how much the source vector needs to be shifted and
+/// what byte number needs to be specified for the instruction to put the
+/// element in the desired location of the target vector.
+bool PPC::isXXINSERTWMask(ShuffleVectorSDNode *N, unsigned &ShiftElts,
+                          unsigned &InsertAtByte, bool &Swap, bool IsLE) {
+
----------------
From the coding guidelines:
"Don’t duplicate the documentation comment in the header file and in the implementation file. Put the documentation comments for public APIs into the header file. Documentation comments for private APIs can go to the implementation file. In any case, implementation files can include additional comments (not necessarily in Doxygen markup) to explain implementation details as needed."

I think you should remove these comments, and leave them in the header file.

================
Comment at: lib/Target/PowerPC/PPCISelLowering.cpp:1508
@@ +1507,3 @@
+bool PPC::isXXINSERTWMask(ShuffleVectorSDNode *N, unsigned &ShiftElts,
+                          unsigned &InsertAtByte, bool &Swap, bool IsLE) {
+
----------------
Can this method be marked as const?


Repository:
  rL LLVM

http://reviews.llvm.org/D20239