[PATCH] D109902: [PowerPC] Improved codegen related to xscvdpsxws/xscvdpuxws

[Amy Kwan via Phabricator via llvm-commits]

amyk added inline comments.


================
Comment at: llvm/lib/Target/PowerPC/PPCInstrVSX.td:2815
+def : Pat<(v4i32 (PPCSToV DblToUInt.A)),
+          (v4i32 (SUBREG_TO_REG (i64 1), (XSCVDPSXWS f64:$A), sub_64))>;
 defm : ScalToVecWPermute<
----------------
This should be `XSCVDPUXWS`?


================
Comment at: llvm/test/CodeGen/PowerPC/test-vector-insert.ll:2
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; xscvdpsxws and uxws is only available on Power7 and above
+; RUN: llc -verify-machineinstrs -mtriple=powerpc64le-unknown-linux-gnu \
----------------
nit: Move this comment under the RUN lines.


================
Comment at: llvm/test/CodeGen/PowerPC/test-vector-insert.ll:3
+; xscvdpsxws and uxws is only available on Power7 and above
+; RUN: llc -verify-machineinstrs -mtriple=powerpc64le-unknown-linux-gnu \
+; RUN:  -mcpu=pwr8 < %s | FileCheck %s --check-prefix=CHECK-LE
----------------
I'm not sure why P8 is LE run only line, and P7 is BE run line only. 
Maybe we should have LE/BE run lines for both P7 and P8 for more coverage. 
Furthermore, if both the LE/BE checks end up the same, we can do `CHECK-P7` and `CHECK-P8`. 

Also, since this looks like it's a Linux test, please add `-ppc-asm-full-reg-names -ppc-vsr-nums-as-vr`.


================
Comment at: llvm/test/CodeGen/PowerPC/vec_conv_fp64_to_i32_elts.ll:16
 ; CHECK-P8-NEXT:    xxswapd vs0, v2
-; CHECK-P8-NEXT:    xscvdpuxws f1, v2
-; CHECK-P8-NEXT:    xscvdpuxws f0, f0
-; CHECK-P8-NEXT:    mffprwz r3, f1
-; CHECK-P8-NEXT:    mtvsrwz v2, r3
-; CHECK-P8-NEXT:    mffprwz r4, f0
-; CHECK-P8-NEXT:    mtvsrwz v3, r4
+; CHECK-P8-NEXT:    xscvdpsxws v2, v2
+; CHECK-P8-NEXT:    xscvdpsxws v3, f0
----------------
This is an unsigned test case, so should be `xscvdpuxws`, right?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D109902/new/

https://reviews.llvm.org/D109902