[PATCH] D40497: [PowerPC] Partially enable the ISEL expansion pass.
Nemanja Ivanovic via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Fri Dec 8 12:50:06 PST 2017
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
More information about the llvm-commits
mailing list