[PATCH] D40497: [PowerPC] Partially enable the ISEL expansion pass.

[Nemanja Ivanovic via Phabricator via llvm-commits]

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

LGTM. Feel free to address the comments on the commit.



================
Comment at: lib/Target/PowerPC/PPCExpandISEL.cpp:213
+        NumRemoved++;
+        (*I)->eraseFromParent();
+        I++;
----------------
Please add a comment such as
```
// FIXME: if the CR field used has no other uses, we could eliminate the
// instruction that defines it. This would have to be done manually since
// this pass runs too late to run DCE after it.
```


================
Comment at: lib/Target/PowerPC/PPCExpandISEL.cpp:224
+        NumFolded++;
+        BuildMI(*MBB, (*I), dl, TII->get(isISEL8(**I) ? PPC::OR8 : PPC::OR))
+            .add(Dest)
----------------
Just add a comment here that you're using both the `TrueValue` and `FalseValue` operands so as not to lose the kill flag if it is set on either of them.


================
Comment at: lib/Target/PowerPC/PPCExpandISEL.cpp:230
+        I++;
+      } else { // Normal cases
+        if (ExpandISELEnabled) {
----------------
I think it's a little more clear/readable if you just make this an `else if(...) {...}` rather than an `if` nested in the else block.


https://reviews.llvm.org/D40497