[PowerPC] Improved codegen related to xscvdpsxws/xscvdpuxws

[Conanap]

Summary

This patch removes the uneccessary mf/mtvsr generated in conjunction
with xscvdpsxws/xscvdpuxws.

[Conanap]

Removed unintended change, moved pattern to more appropriate location, reduced added complexity to 600 as it still works.

[Conanap]

Removed complexity and restored a test case

[kamaub]

This patch seems almost ready to land to me, I'm just a bit concerned about the testing coverage,
is the little endian testing case suppose to target pwr7 as the big endian test does? The default
FileCheck line seems redundant to me, did I misunderstand?

llvm/test/CodeGen/PowerPC/test-vector-insert.ll

9

It seems like this run line is redundant, it produces the same assembly as the big endian specific line above. Maybe the -mcpu=pwr7 can be moved to the first Little-endian specific run line? That line currently only test the target cpu of the test machine.

[Conanap]

Removed AIX test line as it has the same code gen as BE

[amyk]

llvm/lib/Target/PowerPC/PPCInstrVSX.td

2815

This should be XSCVDPUXWS?

llvm/test/CodeGen/PowerPC/test-vector-insert.ll

2

nit: Move this comment under the RUN lines.

3

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.

llvm/test/CodeGen/PowerPC/vec_conv_fp64_to_i32_elts.ll

16

This is an unsigned test case, so should be xscvdpuxws, right?

[Conanap]

Updated test cases, fixed a typo

[Conanap]

llvm/test/CodeGen/PowerPC/test-vector-insert.ll

9

good point, thanks!

[Conanap]

Added P7 and P8 run lines for BE

[amyk]

LGTM. Thanks for addressing comments.

[nemanjai]

There isn't enough information in the test to determine whether this adequately improves code generation (in fact, whether it improves code generation at all).

llvm/test/CodeGen/PowerPC/test-vector-insert.ll

2

Test cases where we improve code generation should be pre-committed as an NFC change so that the review shows just the differences in code generation. It is not easy to evaluate whether the patch improves the code or not without seeing how it changed.

6

Code generation on Power9 is important as well. Please add that test.

16

nit: s/Codgen/Codegen

[Conanap]

This modifies a test case introduced in this commit: https://github.com/llvm/llvm-project/commit/3678df5ae6618eec656ae0ea0dab3be09d73bc9a

[nemanjai]

I cannot tell without context whether the codegen changes for the f32->i32 convert + insert case (for example @test2 in test/CodeGen/PowerPC/test-vector-insert.ll) but it should change.

llvm/lib/Target/PowerPC/PPCInstrVSX.td

4145

I assume this is the big endian Power9 block (and the below is the little endian). However, I can't confirm that since the context for the patch has disappeared somehow.

4150

What am I missing here? It appears to me that this pattern is exactly the same as the one above it. The same appears to be the case for all of these. Also, why the change in naming convention?

[Conanap]

Added context, changed var names in aptterns

[Conanap]

llvm/lib/Target/PowerPC/PPCInstrVSX.td

4150

This one is for the unsigned version, the above is for the signed version. Originally, using DblToInt.A gave me problems as $A are now used in two places, so I've changed the other variable names instead. I'll use DblToInt.B instead to match the convention, thanks!

[nemanjai]

I really don't understand what happened now. It seems that you have simply reverted to an older version of this patch. The test case appears to not have been pre-committed any longer, the Power9 patterns and test cases are gone. What has happened here?

[Conanap]

Updated correct version of the patch

[nemanjai]

LGTM.

[amyk]

Additional nits regarding comments.

llvm/test/CodeGen/PowerPC/test-vector-insert.ll

20

21

llvm/lib/Target/PowerPC/PPCInstrVSX.td

def : Pat<(v4i32 (build_vector DblToUInt.A, DblToUInt.A,
                                DblToUInt.A, DblToUInt.A)),
           (v4i32 (XXSPLTW (SUBREG_TO_REG (i64 1), (XSCVDPUXWS $A), sub_64), 1))>;
 def : Pat<(v2i64 (build_vector DblToLong.A, DblToLong.A)),
           (v2i64 (XXPERMDI (SUBREG_TO_REG (i64 1), (XSCVDPSXDS $A), sub_64),
                            (SUBREG_TO_REG (i64 1), (XSCVDPSXDS $A), sub_64), 0))>;
 def : Pat<(v2i64 (build_vector DblToULong.A, DblToULong.A)),
           (v2i64 (XXPERMDI (SUBREG_TO_REG (i64 1), (XSCVDPUXDS $A), sub_64),
                            (SUBREG_TO_REG (i64 1), (XSCVDPUXDS $A), sub_64), 0))>;
+def : Pat<(v4i32 (PPCSToV DblToInt.A)),
+          (v4i32 (SUBREG_TO_REG (i64 1), (XSCVDPSXWS f64:$A), sub_64))>;
+def : Pat<(v4i32 (PPCSToV DblToUInt.A)),
+          (v4i32 (SUBREG_TO_REG (i64 1), (XSCVDPSXWS f64:$A), sub_64))>;
 defm : ScalToVecWPermute<
   v4i32, FltToIntLoad.A,
   (XXSPLTW (SUBREG_TO_REG (i64 1), (XSCVDPSXWSs (XFLOADf32 ForceXForm:$A)), sub_64), 1),
   (SUBREG_TO_REG (i64 1), (XSCVDPSXWSs (XFLOADf32 ForceXForm:$A)), sub_64)>;
 defm : ScalToVecWPermute<
   v4i32, FltToUIntLoad.A,
   (XXSPLTW (SUBREG_TO_REG (i64 1), (XSCVDPUXWSs (XFLOADf32 ForceXForm:$A)), sub_64), 1),
   (SUBREG_TO_REG (i64 1), (XSCVDPUXWSs (XFLOADf32 ForceXForm:$A)), sub_64)>;

 def : Pat<(f64 (PPCfcfidu (f64 (PPCmtvsrz (i32 (extractelt v4i32:$A, 1)))))),
           (f64 (XSCVUXDDP (XXEXTRACTUW $A, 4)))>;
 def : Pat<(f64 (PPCfcfidu (f64 (PPCmtvsrz (i32 (extractelt v4i32:$A, 2)))))),
           (f64 (XSCVUXDDP (XXEXTRACTUW $A, 8)))>;
 def : Pat<(f64 (PPCfcfidu (f64 (PPCmtvsrz (i32 (extractelt v4i32:$A, 3)))))),
           (f64 (XSCVUXDDP (XXEXTRACTUW $A, 12)))>;
 def : Pat<(v4i32 (insertelt v4i32:$A, i32:$B, 0)),
           (v4i32 (XXINSERTW v4i32:$A, AlignValues.I32_TO_BE_WORD1, 0))>;
 def : Pat<(v4i32 (insertelt v4i32:$A, i32:$B, 1)),
           (v4i32 (XXINSERTW v4i32:$A, AlignValues.I32_TO_BE_WORD1, 4))>;
 def : Pat<(v4i32 (insertelt v4i32:$A, i32:$B, 2)),
           (v4i32 (XXINSERTW v4i32:$A, AlignValues.I32_TO_BE_WORD1, 8))>;
 def : Pat<(v4i32 (insertelt v4i32:$A, i32:$B, 3)),
           (v4i32 (XXINSERTW v4i32:$A, AlignValues.I32_TO_BE_WORD1, 12))>;
 def : Pat<(v4f32 (insertelt v4f32:$A, f32:$B, 0)),
           (v4f32 (XXINSERTW v4f32:$A, AlignValues.F32_TO_BE_WORD1, 0))>;
 def : Pat<(v4f32 (insertelt v4f32:$A, f32:$B, 1)),
           (v4f32 (XXINSERTW v4f32:$A, AlignValues.F32_TO_BE_WORD1, 4))>;
 def : Pat<(v4f32 (insertelt v4f32:$A, f32:$B, 2)),
           (v4f32 (XXINSERTW v4f32:$A, AlignValues.F32_TO_BE_WORD1, 8))>;
 def : Pat<(v4f32 (insertelt v4f32:$A, f32:$B, 3)),
           (v4f32 (XXINSERTW v4f32:$A, AlignValues.F32_TO_BE_WORD1, 12))>;

llvm/test/CodeGen/PowerPC/test-vector-insert.ll

+; 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 \
+; RUN:  -mcpu=pwr8 < %s | FileCheck %s --check-prefix=CHECK-LE
+; RUN: llc -verify-machineinstrs -mtriple=powerpc64-unknown-linux-gnu \
+; RUN:  -mcpu=pwr7 < %s | FileCheck %s --check-prefix=CHECK-BE
+
+define dso_local <4 x i32> @test(<4 x i32> %a, double %b) {
+; CHECK-LE-LABEL: test:
+; CHECK-LE:       # %bb.0: # %entry
+; CHECK-LE-NEXT:    xscvdpsxws 35, 1
+; CHECK-LE-NEXT:    addis 3, 2, .LCPI0_0@toc@ha
+; CHECK-LE-NEXT:    addi 3, 3, .LCPI0_0@toc@l
+; CHECK-LE-NEXT:    lvx 4, 0, 3
+; CHECK-LE-NEXT:    vperm 2, 3, 2, 4
+; CHECK-LE-NEXT:    blr
+;
+; CHECK-BE-LABEL: test:
+; CHECK-BE:       # %bb.0: # %entry
+; CHECK-BE-NEXT:    xscvdpsxws 0, 1
+; CHECK-BE-NEXT:    addi 3, 1, -4
+; CHECK-BE-NEXT:    stfiwx 0, 0, 3
+; CHECK-BE-NEXT:    lwz 3, -4(1)
+; CHECK-BE-NEXT:    xxsldwi 0, 34, 34, 3
+; CHECK-BE-NEXT:    stw 3, -32(1)
+; CHECK-BE-NEXT:    addi 3, 1, -32
+; CHECK-BE-NEXT:    lxvw4x 1, 0, 3
+; CHECK-BE-NEXT:    xxsldwi 34, 0, 1, 1
+; CHECK-BE-NEXT:    blr
+entry:
+  %conv = fptosi double %b to i32
+  %vecins = insertelement <4 x i32> %a, i32 %conv, i32 3
+  ret <4 x i32> %vecins
+}
+
+define dso_local <4 x i32> @test2(<4 x i32> %a, float %b) {
+; CHECK-LE-LABEL: test2:
+; CHECK-LE:       # %bb.0: # %entry
+; CHECK-LE-NEXT:    xscvdpsxws 35, 1
+; CHECK-LE-NEXT:    addis 3, 2, .LCPI1_0@toc@ha
+; CHECK-LE-NEXT:    addi 3, 3, .LCPI1_0@toc@l
+; CHECK-LE-NEXT:    lvx 4, 0, 3
+; CHECK-LE-NEXT:    vperm 2, 3, 2, 4
+; CHECK-LE-NEXT:    blr
+;
+; CHECK-BE-LABEL: test2:
+; CHECK-BE:       # %bb.0: # %entry
+; CHECK-BE-NEXT:    xscvdpsxws 0, 1
+; CHECK-BE-NEXT:    addi 3, 1, -4
+; CHECK-BE-NEXT:    stfiwx 0, 0, 3
+; CHECK-BE-NEXT:    lwz 3, -4(1)
+; CHECK-BE-NEXT:    xxsldwi 0, 34, 34, 3
+; CHECK-BE-NEXT:    stw 3, -32(1)
+; CHECK-BE-NEXT:    addi 3, 1, -32
+; CHECK-BE-NEXT:    lxvw4x 1, 0, 3
+; CHECK-BE-NEXT:    xxsldwi 34, 0, 1, 1
+; CHECK-BE-NEXT:    blr
+entry:
+  %conv = fptosi float %b to i32
+  %vecins = insertelement <4 x i32> %a, i32 %conv, i32 3
+  ret <4 x i32> %vecins
+}
+
+define dso_local <4 x i32> @test3(<4 x i32> %a, double %b) {
+; CHECK-LE-LABEL: test3:
+; CHECK-LE:       # %bb.0: # %entry
+; CHECK-LE-NEXT:    xscvdpsxws 35, 1
+; CHECK-LE-NEXT:    addis 3, 2, .LCPI2_0@toc@ha
+; CHECK-LE-NEXT:    addi 3, 3, .LCPI2_0@toc@l
+; CHECK-LE-NEXT:    lvx 4, 0, 3
+; CHECK-LE-NEXT:    vperm 2, 3, 2, 4
+; CHECK-LE-NEXT:    blr
+;
+; CHECK-BE-LABEL: test3:
+; CHECK-BE:       # %bb.0: # %entry
+; CHECK-BE-NEXT:    xscvdpuxws 0, 1
+; CHECK-BE-NEXT:    addi 3, 1, -4
+; CHECK-BE-NEXT:    stfiwx 0, 0, 3
+; CHECK-BE-NEXT:    lwz 3, -4(1)
+; CHECK-BE-NEXT:    xxsldwi 0, 34, 34, 3
+; CHECK-BE-NEXT:    stw 3, -32(1)
+; CHECK-BE-NEXT:    addi 3, 1, -32
+; CHECK-BE-NEXT:    lxvw4x 1, 0, 3
+; CHECK-BE-NEXT:    xxsldwi 34, 0, 1, 1
+; CHECK-BE-NEXT:    blr
+entry:
+  %conv = fptoui double %b to i32
+  %vecins = insertelement <4 x i32> %a, i32 %conv, i32 3
+  ret <4 x i32> %vecins
+}
+
+define dso_local <4 x i32> @test4(<4 x i32> %a, float %b) {
+; CHECK-LE-LABEL: test4:
+; CHECK-LE:       # %bb.0: # %entry
+; CHECK-LE-NEXT:    xscvdpsxws 35, 1
+; CHECK-LE-NEXT:    addis 3, 2, .LCPI3_0@toc@ha
+; CHECK-LE-NEXT:    addi 3, 3, .LCPI3_0@toc@l
+; CHECK-LE-NEXT:    lvx 4, 0, 3
+; CHECK-LE-NEXT:    vperm 2, 3, 2, 4
+; CHECK-LE-NEXT:    blr
+;
+; CHECK-BE-LABEL: test4:
+; CHECK-BE:       # %bb.0: # %entry
+; CHECK-BE-NEXT:    xscvdpuxws 0, 1
+; CHECK-BE-NEXT:    addi 3, 1, -4
+; CHECK-BE-NEXT:    stfiwx 0, 0, 3
+; CHECK-BE-NEXT:    lwz 3, -4(1)
+; CHECK-BE-NEXT:    xxsldwi 0, 34, 34, 3
+; CHECK-BE-NEXT:    stw 3, -32(1)
+; CHECK-BE-NEXT:    addi 3, 1, -32
+; CHECK-BE-NEXT:    lxvw4x 1, 0, 3
+; CHECK-BE-NEXT:    xxsldwi 34, 0, 1, 1
+; CHECK-BE-NEXT:    blr
+entry:
+  %conv = fptoui float %b to i32
+  %vecins = insertelement <4 x i32> %a, i32 %conv, i32 3
+  ret <4 x i32> %vecins
+}

llvm/test/CodeGen/PowerPC/vec_conv_fp64_to_i32_elts.ll

 ; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
 ; RUN: llc -verify-machineinstrs -mtriple=powerpc64le-unknown-linux-gnu \
 ; RUN:     -mcpu=pwr8 -ppc-asm-full-reg-names -ppc-vsr-nums-as-vr < %s | \
 ; RUN: FileCheck %s --check-prefix=CHECK-P8
 ; RUN: llc -verify-machineinstrs -mtriple=powerpc64le-unknown-linux-gnu \
 ; RUN:     -mcpu=pwr9 -ppc-asm-full-reg-names -ppc-vsr-nums-as-vr < %s | \
 ; RUN: FileCheck %s --check-prefix=CHECK-P9
 ; RUN: llc -verify-machineinstrs -mtriple=powerpc64-unknown-linux-gnu \
 ; RUN:     -mcpu=pwr9 -ppc-asm-full-reg-names -ppc-vsr-nums-as-vr < %s | \
 ; RUN: FileCheck %s --check-prefix=CHECK-BE
 
 define i64 @test2elt(<2 x double> %a) local_unnamed_addr #0 {
 ; CHECK-P8-LABEL: test2elt:
 ; CHECK-P8:       # %bb.0: # %entry
 ; 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
 ; CHECK-P8-NEXT:    vmrghw v2, v2, v3
 ; CHECK-P8-NEXT:    xxswapd vs0, v2
 ; CHECK-P8-NEXT:    mffprd r3, f0
 ; CHECK-P8-NEXT:    blr
 ;
 ; CHECK-P9-LABEL: test2elt:
 ; CHECK-P9:       # %bb.0: # %entry
-; CHECK-P9-NEXT:    xscvdpuxws f0, v2
-; CHECK-P9-NEXT:    mffprwz r3, f0
 ; CHECK-P9-NEXT:    xxswapd vs0, v2
-; CHECK-P9-NEXT:    mtvsrwz v3, r3
-; CHECK-P9-NEXT:    xscvdpuxws f0, f0
-; CHECK-P9-NEXT:    mffprwz r3, f0
-; CHECK-P9-NEXT:    mtvsrwz v2, r3
+; CHECK-P9-NEXT:    xscvdpsxws v3, v2
+; CHECK-P9-NEXT:    xscvdpsxws v2, f0
 ; CHECK-P9-NEXT:    vmrghw v2, v3, v2
 ; CHECK-P9-NEXT:    mfvsrld r3, v2
 ; CHECK-P9-NEXT:    blr
 ;
 ; CHECK-BE-LABEL: test2elt:
 ; CHECK-BE:       # %bb.0: # %entry
-; CHECK-BE-NEXT:    xscvdpuxws f0, v2
-; CHECK-BE-NEXT:    mffprwz r3, f0
 ; CHECK-BE-NEXT:    xxswapd vs0, v2
-; CHECK-BE-NEXT:    mtvsrwz v3, r3
-; CHECK-BE-NEXT:    xscvdpuxws f0, f0
-; CHECK-BE-NEXT:    mffprwz r3, f0
-; CHECK-BE-NEXT:    mtvsrwz v2, r3
+; CHECK-BE-NEXT:    xscvdpsxws v3, v2
+; CHECK-BE-NEXT:    xscvdpsxws v2, f0
 ; CHECK-BE-NEXT:    vmrgow v2, v3, v2
 ; CHECK-BE-NEXT:    mfvsrd r3, v2
 ; CHECK-BE-NEXT:    blr
 entry:
   %0 = fptoui <2 x double> %a to <2 x i32>
   %1 = bitcast <2 x i32> %0 to i64
   ret i64 %1
 }
entry:
   store <16 x i32> %1, <16 x i32>* %agg.result, align 64
   ret void
 }
 
 define i64 @test2elt_signed(<2 x double> %a) local_unnamed_addr #0 {
 ; CHECK-P8-LABEL: test2elt_signed:
 ; CHECK-P8:       # %bb.0: # %entry
 ; CHECK-P8-NEXT:    xxswapd vs0, v2
-; CHECK-P8-NEXT:    xscvdpsxws f1, v2
-; CHECK-P8-NEXT:    xscvdpsxws 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
 ; CHECK-P8-NEXT:    vmrghw v2, v2, v3
 ; CHECK-P8-NEXT:    xxswapd vs0, v2
 ; CHECK-P8-NEXT:    mffprd r3, f0
 ; CHECK-P8-NEXT:    blr
 ;
 ; CHECK-P9-LABEL: test2elt_signed:
 ; CHECK-P9:       # %bb.0: # %entry
-; CHECK-P9-NEXT:    xscvdpsxws f0, v2
-; CHECK-P9-NEXT:    mffprwz r3, f0
 ; CHECK-P9-NEXT:    xxswapd vs0, v2
-; CHECK-P9-NEXT:    mtvsrwz v3, r3
-; CHECK-P9-NEXT:    xscvdpsxws f0, f0
-; CHECK-P9-NEXT:    mffprwz r3, f0
-; CHECK-P9-NEXT:    mtvsrwz v2, r3
+; CHECK-P9-NEXT:    xscvdpsxws v3, v2
+; CHECK-P9-NEXT:    xscvdpsxws v2, f0
 ; CHECK-P9-NEXT:    vmrghw v2, v3, v2
 ; CHECK-P9-NEXT:    mfvsrld r3, v2
 ; CHECK-P9-NEXT:    blr
 ;
 ; CHECK-BE-LABEL: test2elt_signed:
 ; CHECK-BE:       # %bb.0: # %entry
-; CHECK-BE-NEXT:    xscvdpsxws f0, v2
-; CHECK-BE-NEXT:    mffprwz r3, f0
 ; CHECK-BE-NEXT:    xxswapd vs0, v2
-; CHECK-BE-NEXT:    mtvsrwz v3, r3
-; CHECK-BE-NEXT:    xscvdpsxws f0, f0
-; CHECK-BE-NEXT:    mffprwz r3, f0
-; CHECK-BE-NEXT:    mtvsrwz v2, r3
+; CHECK-BE-NEXT:    xscvdpsxws v3, v2
+; CHECK-BE-NEXT:    xscvdpsxws v2, f0
 ; CHECK-BE-NEXT:    vmrgow v2, v3, v2
 ; CHECK-BE-NEXT:    mfvsrd r3, v2
 ; CHECK-BE-NEXT:    blr
 entry:
   %0 = fptosi <2 x double> %a to <2 x i32>
   %1 = bitcast <2 x i32> %0 to i64
   ret i64 %1
 }