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

[Hiroshi Inoue via Phabricator via llvm-commits]

inouehrs added inline comments.


================
Comment at: lib/Target/PowerPC/PPCInstrInfo.cpp:1633
   // Scan forward to find the first use of the compare.
+  bool FoundUse = false;
   for (MachineBasicBlock::iterator EL = CmpInstr.getParent()->end(); I != EL;
----------------
nemanjai wrote:
> This change doesn't seem meaningful but it's harmless, so I guess it's OK.
In the first patch, I was reusing `FoundUse` outside the loop and so I moved the definition of `FoundUse` before the loop. The current version does not use this outside loop.


================
Comment at: lib/Target/PowerPC/PPCInstrInfo.cpp:1664
   else if (MI->getParent() != CmpInstr.getParent() || Value != 0) {
+    if (FoundUse && !equalityOnly) {
+      MachineInstr *UseMI = &*I;
----------------
nemanjai wrote:
> It isn't clear from this code how it addresses the concern in the comment. Besides, the comment should be updated if this code alleviates the cross-call concern.
> Of course, I haven't checked all the code in this function but this code does not appear to do anything to address a situation where there might be a call between the instruction that will be converted to record form and the actual use of the CR field.
I modified the code to be conservative on checking MI and CmpInstr are in the same BB. Now, the optimization on the comparison against 1 or -1 is applied only when Value != 0 and MI and CmpInstr are in the same BB.


https://reviews.llvm.org/D30081