[PATCH] D33404: [PowerPC] Fix a performance bug for PPC::XXPERMDI.

[Kit Barton via Phabricator via llvm-commits]

kbarton added inline comments.


================
Comment at: lib/Target/PowerPC/PPCISelLowering.cpp:1671
+   unsigned B[8];
+  for (unsigned i = 0; i < 2; ++i) {
+    B[0] = N->getMaskElt(i * 8);
----------------
nemanjai wrote:
> I'm not a fan of loops with 2 iterations. It's probably clearer if this is just straight-line code.
I agree - this should be just straight line code, with a short comment explaining the logic. 


================
Comment at: lib/Target/PowerPC/PPCISelLowering.cpp:1747
+
+bool PPC::isXXPERMDIShuffleMask(ShuffleVectorSDNode *N, unsigned &DM,
+                               bool &Swap, bool IsLE) {
----------------
Please add a comment explaining the semantics, and what what conditions the parameters will be modified.



================
Comment at: lib/Target/PowerPC/PPCISelLowering.cpp:1782
+
+    DM = getDMValue(M0, M1, IsLE);
+    return true;
----------------
This is a bit hard to follow, but I don't think Swap is modified on this path. Is that intentional? If so, it needs to be documented clearly in the comments above. 


================
Comment at: lib/Target/PowerPC/PPCISelLowering.cpp:1794
+
+    DM = getDMValue(M0, M1, IsLE);
+    return true;
----------------
Same comment for Swap. 


================
Comment at: lib/Target/PowerPC/PPCISelLowering.h:457
                               bool &Swap, bool IsLE);
+    bool isXXPERMDIShuffleMask(ShuffleVectorSDNode *N, unsigned &ShiftElts,
+                              bool &Swap, bool IsLE);
----------------
Please add a comment above to be consistent with all of the other declarations here. 


https://reviews.llvm.org/D33404