[PATCH] D36734: [PowerPC Peephole] Constants into a join add, use ADDI over LI/ADD.

[Hiroshi Inoue via Phabricator via llvm-commits]

inouehrs added inline comments.


================
Comment at: lib/Target/PowerPC/PPCMIPeephole.cpp:96
 
+MachineInstr *getVRegDefOrNull(MachineOperand *Op, MachineRegisterInfo *MRI) {
+  assert(Op && "Invalid Operand!");
----------------
It's better to make this local helper method static.


================
Comment at: lib/Target/PowerPC/PPCMIPeephole.cpp:366
+
+      case PPC::ADD4:
+      case PPC::ADD8: {
----------------
Why specific to add?
Don't we have much optimization opportunity for arithmetic operations other than add?


================
Comment at: lib/Target/PowerPC/PPCMIPeephole.cpp:375
+
+          if (!MRI->hasOneNonDBGUse(DefPhiMI->getOperand(0).getReg()))
+            return false;
----------------
```
return DefPhiMI && (DefPhiMI->getOpcode() != PPC::PHI &&
       MRI->hasOneNonDBGUse(DefPhiMI->getOperand(0).getReg());
```
might be clearer (but its up to your preference.)


================
Comment at: lib/Target/PowerPC/PPCMIPeephole.cpp:407
+        else if (!isSingleUsePHI(&Op1) || !dominatesAllSingleUseLIs(&Op2, &Op1))
+          break; // We don't have and ADD fed by LI's that can be transformed
+
----------------
We don't have and ADD fed -> redundant "and"?


================
Comment at: lib/Target/PowerPC/PPCMIPeephole.cpp:409
+
+        // Now we now that Op1 is the PHI node and Op2 is the dominator
+        unsigned DominatorReg = Op2.getReg();
----------------
Now we now -> do you mean "Now we know"?


https://reviews.llvm.org/D36734