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

[Nemanja Ivanovic via llvm-commits]

nemanjai added inline comments.

================
Comment at: lib/Target/PowerPC/PPCISelDAGToDAG.cpp:245
@@ -244,1 +244,3 @@
   };
+
+  struct SelectCCSummary {
----------------
Perhaps a short comment describing what the purpose of this struct is.

================
Comment at: lib/Target/PowerPC/PPCISelDAGToDAG.cpp:498
@@ +497,3 @@
+
+    switch (CC) {
+       case ISD::CondCode::SETNE:
----------------
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.

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

================
Comment at: lib/Target/PowerPC/PPCISelDAGToDAG.cpp:537
@@ +536,3 @@
+     Summary.IsStrict = false;
+     switch (CC0) {
+       case ISD::CondCode::SETNE:
----------------
Same comment about fall-through.

================
Comment at: test/CodeGen/PowerPC/vsx-p9.ll:2
@@ +1,3 @@
+; RUN: llc -mcpu=pwr9 -mattr=+power9-vector < %s | FileCheck %s
+
+attributes #1 = {"no-nans-fp-math"="true"}
----------------
cycheng wrote:
> Need define:
>   target triple = "powerpc64-unknown-linux-gnu"
> 
> or:
>   llc -mtriple=powerpc64le-unknown-linux-gnu
Yes, the latter please. You should always specify the triple because I think other targets will get the "pwr9 is not a valid CPU for this target" message if you don't.


http://reviews.llvm.org/D20019