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

Ehsan Amiri via llvm-commits llvm-commits at lists.llvm.org
Wed May 18 11:34:30 PDT 2016


amehsan added inline comments.

================
Comment at: lib/Target/PowerPC/PPCISelDAGToDAG.cpp:498
@@ +497,3 @@
+
+    switch (CC) {
+       case ISD::CondCode::SETNE:
----------------
nemanjai wrote:
> Although the fall-through seems reasonable here, I think it's a good idea to add comments to that end. I'm not sure if everyone will agree with me though. So maybe others can chime in here as well.
Are you in doubt about functional correctness of fall-through or something else? functional correctness should be covered in the testcases. I will think about this again, to see if there are missing patterns in the testcases.

================
Comment at: lib/Target/PowerPC/PPCISelDAGToDAG.cpp:528
@@ +527,3 @@
+                             (N->getOperand(0).getOperand(1).
+                                               getOperand(2))->get();
+
----------------
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.


http://reviews.llvm.org/D20019





More information about the llvm-commits mailing list