[PATCH] D33225: [PowerPC] Fix a performance bug for PPC::XXSLDWI.

Nemanja Ivanovic via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue May 23 14:42:12 PDT 2017


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

The remaining comments are minor. Please address them on the commit.

LGTM but you may want to give the other reviewers a bit of time to take a look as well.



================
Comment at: lib/Target/PowerPC/PPCISelLowering.cpp:1669
+                               bool &Swap, bool IsLE) {
+  if (N->getValueType(0) != MVT::v16i8)
+    return false;
----------------
I think this can safely be an assert.


================
Comment at: lib/Target/PowerPC/PPCISelLowering.cpp:1685
+  if (N->getOperand(1).isUndef()) {
+    if (M0 != 0 && M0 != 1 && M0 != 2 && M0 != 3)
+      return false;
----------------
This can be converted to an assert. Something like:
`assert(M0 < 4 && "Indexing into an undef vector?");`


================
Comment at: lib/Target/PowerPC/PPCISelLowering.cpp:1701
+  if (IsLE) {
+    if (M0 == 0 || M0 == 7 || M0 == 6 || M0 == 5) {
+      Swap = false;
----------------
A comment to clarify this is needed. Perhaps something like:
```
// Input vectors don't need to be swapped if the leading element
// of the result is one of the 3 left elements of the second vector
// (or if there is no shift to be done at all).
```

Also, add similar comments to the BE code.


================
Comment at: lib/Target/PowerPC/PPCISelLowering.cpp:1704
+      ShiftElts = (8 - M0) % 8;
+    } else if (M0 == 4 || M0 == 3 || M0 == 2 || M0 == 1) {
+      Swap = true;
----------------
```
// Input vectors need to be swapped if the leading element
// of the result is one of the 3 left elements of the first vector
// (or if we're shifting by 4 - thereby simply swapping the vectors).
```


https://reviews.llvm.org/D33225





More information about the llvm-commits mailing list