[PATCH] D20019: [PPC] exploitation of new xscmp*, as well as xsmaxcdp and xsmincdp

[Ehsan Amiri via llvm-commits]

amehsan added inline comments.

================
Comment at: lib/Target/PowerPC/PPCISelDAGToDAG.cpp:528
@@ +527,3 @@
+                             (N->getOperand(0).getOperand(1).
+                                               getOperand(2))->get();
+
----------------
nemanjai wrote:
> amehsan wrote:
> > nemanjai wrote:
> > > Is it impossible that these operands do not exist? Namely, is it not possible that operand 1 of N does not have 3 operands thereby causing this call to assert for trying to get an invalid operand? Both here and below.
> > N has an opernad(0) because it is a select_cc. we have checked that N->getOperand(0).getOpcode() is and ISD::AND so it has operand (1). and we have checked that both N->getOperand(0).getOperand(0) and N->getOperand(0).getOperand(1) are SETCC so it has operand 0, 1 and 2.
> OK, excellent. I just didn't look through the early exit out of this conditional branch in detail. Although that brings me to a point I was initially going to post about the if statement above. I find that a descriptive comment for such involved conditional statements is invaluable.
> 
> Overall, it might be nice for this function to have a comment at the top describing all the kinds of DAGs it handles. I understand that we don't comment every possible DAG combine, but when the logic is not easy and straightforward to follow by reading the code, I find a descriptive comment goes a long way for readability.
Sure, will add more comments to the code.


http://reviews.llvm.org/D20019