[PATCH] D39860: [PowerPC] Simplify a Swap if it feeds a Splat

[Nemanja Ivanovic via Phabricator via llvm-commits]

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

This looks good, but of course, lets run this through all the testing before committing. Also, I think that for the test cases you added, you don't expect to see any swaps. You should be able to add `-implicit-check-not` to the RUN lines.
With these minor changes, LGTM.



================
Comment at: lib/Target/PowerPC/PPCMIPeephole.cpp:319
         unsigned MyOpcode = MI.getOpcode();
         unsigned OpNo = MyOpcode == PPC::XXSPLTW ? 1 : 2;
         unsigned TrueReg = lookThruCopyLike(MI.getOperand(OpNo).getReg());
----------------
Since you're adding `ImmOpNo` as well, please rename this to `RegOpNo` to make it clear. I know we generally don't rename things in the patch like this, but I think it's OK here because it is the introduction of the new one that requires this one to be renamed.


================
Comment at: lib/Target/PowerPC/PPCMIPeephole.cpp:359
+        unsigned ImmOpNo = MyOpcode == PPC::XXSPLTW ? 2 : 1;
+        if (!MI.getOperand(ImmOpNo).isImm()) {
+          DEBUG(dbgs() << "Expected op number: " << ImmOpNo <<
----------------
It's fine to just get rid of this and turn it into an assert. Just assert that the expected operand is an immediate.


================
Comment at: lib/Target/PowerPC/PPCMIPeephole.cpp:382
+          unsigned DefOp2 = DefMI->getOperand(2).getReg();
+          unsigned DefImm = DefMI->getOperand(3).getImm();
+          unsigned SplatImm = MI.getOperand(ImmOpNo).getImm();
----------------
Add a comment such as:
```
// Note that in both cases, the immediate represents the
// number of words the input vector is rotated left.
// For example: xxsldwi x, y, y, 2 == xxpermdi x, y, y, 2.
```


================
Comment at: lib/Target/PowerPC/PPCMIPeephole.cpp:385
+
+          // Check that the two ops are equal.
+          if (DefOp1 != DefOp2)
----------------
```
// If the two register operands differ, this isn't a
// shift/swap but a permutation of a pair of concatenated
// registers.
```


https://reviews.llvm.org/D39860