[PATCH] D60402: [PowerPC] Collapse RLDICL/RLDICR into RLDIC when possible

[Stefan Pintilie via Phabricator via llvm-commits]

stefanp requested changes to this revision.
stefanp added a comment.
This revision now requires changes to proceed.

The patch does what we want it to do in most cases (including the example that motivated it) but some boundary conditions need to be considered before it is safe in all cases.



================
Comment at: lib/Target/PowerPC/PPCMIPeephole.cpp:51
+STATISTIC(NumRotatesCollapsed,
+          "Number of pairs of rotate left, clear left/right colapsed");
 
----------------
nit:
collapsed


================
Comment at: lib/Target/PowerPC/PPCMIPeephole.cpp:785
+        uint64_t NewSH = SHSrc + SHMI;
+        uint64_t NewMB = MBSrc - SHMI;
+
----------------
The value for NewMB can be negative.
For example:
```
    %1:g8rc = RLDICL %0, 10, 1
    %2:g8rc = RLDICR %1, 10, 43
```
In this case NewMB = -9 
I changed the testcase you provided and used the above two lines and I get an assert.

We may still be able to handle the negative case (because this is a rotate) but it may require another if statement and special handling.



================
Comment at: lib/Target/PowerPC/PPCMIPeephole.cpp:793
+        // The bits cleared with RLDIC are [0, NewMB) and (63-NewSH, 63].
+        if ((63 - NewSH) != MEMI)
+          break;
----------------
The value for `(63 - NewSH)` can also be negative and I think it needs to be a special case.
Each shift has to be less than or equal to 63 but the sum of two shifts can be greater than 63.


================
Comment at: test/CodeGen/PowerPC/collapse-rotates.mir:53
+constants:       []
+machineFunctionInfo: {}
+body:             |
----------------
Had to delete this line on the machine I was running on to get this test to pass...
The error is:
```
error: YAML:53:23: unknown key 'machineFunctionInfo'
machineFunctionInfo: {}
```
I'm not sure what the issue is here but I feel like that line is not universally supported.


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60402/new/

https://reviews.llvm.org/D60402