[PATCH] D47332: [PowerPC] Exploit the vector min/max instructions

[Kewen Lin via Phabricator via llvm-commits]

jedilyn added inline comments.


================
Comment at: lib/Target/PowerPC/PPCInstrAltivec.td:931
+          (v4f32 (VMINFP $src1, $src2))>;
+
 // Shuffles.
----------------
Hi Nemanja, I have one concern on whether these two hardware instructions for vector float point can be perfectly mapped to these two ISDNode. 
As the description of fmaxnum/fminnum //"in the case where a single input is NaN, the non-NaN input is returned.",// 
while the description for the vmaxfp/vminfp in ISA like// "The maximum of +0 and -0 is +0. The maximum of any value and a NaN is a QNaN." //
It looks more suitable for the fmaxnan/fminnan?


================
Comment at: lib/Target/PowerPC/PPCInstrVSX.td:1507
+            (v2i64 (VMINUD (COPY_TO_REGCLASS $src1, VRRC),
+                           (COPY_TO_REGCLASS $src2, VRRC)))>;
 } // AddedComplexity = 400
----------------
I'm not sure why not use similar patterns like VMAXSW in PPCInstrAltivec.td but being located in HasP8Vector scope? Is there some special reasons with COPY_TO_REGCLASS? 


================
Comment at: test/CodeGen/PowerPC/vec-min-max.ll:53
+}
+
+define <2 x i64> @getsmaxi64(<2 x i64> %a, <2 x i64> %b) {
----------------
Some more cases cover sge/sle seems trivial but the coverage is better? 


Repository:
  rL LLVM

https://reviews.llvm.org/D47332