[PATCH] D30081: [PPC] Eliminate more compare instructions using record-form operation

[Tim Shen via Phabricator via llvm-commits]

timshen added inline comments.


================
Comment at: lib/Target/PowerPC/PPCInstrInfo.cpp:1690
+    // PPC does not have a record-form SUBri.
+    if (!NonZeroImmed)
+      return false;
----------------
The name is confusing in this scope. If Value != 0, then it literally means The immediate is not zero.

Combined with the next comment, I suggest to use a better name - `Success` is even better IMO.

Alternatively you can wrap up this scope into a function/lambda, and use the return value. That will eliminate the `Success` variable even further.


================
Comment at: lib/Target/PowerPC/PPCInstrInfo.cpp:1762
   bool ShouldSwap = false;
-  if (Sub) {
+  if (Sub && !NonZeroImmed) {
     ShouldSwap = SrcReg2 != 0 && Sub->getOperand(1).getReg() == SrcReg2 &&
----------------
If we use `(Sub && Value == 0)` here, the scope of NonZeroImmed can be limited to the previous small scope `else if (Value != 0) {`, right?

It seems better to have fewer states in the larger scope.


https://reviews.llvm.org/D30081