[PowerPC] Cust lower fpext v2f32 to v2f64 from extract_subvector v4f32

[lei]

Summary

This is a follow up patch from https://reviews.llvm.org/D57857 to handle extract_subvector v4f32.

For cases where we fpext of v2f32 to v2f64 from extract_subvector we currently generate on P9 the following:

lxv 0, 0(3)
xxsldwi 1, 0, 0, 1
xscvspdpn 2, 0
xxsldwi 3, 0, 0, 3
xxswapd 0, 0
xscvspdpn 1, 1
xscvspdpn 3, 3
xscvspdpn 0, 0
xxmrghd 0, 0, 3
xxmrghd 1, 2, 1
stxv 0, 0(4)
stxv 1, 0(5)

This patch custom lower it to the following sequence:

        lxv 0, 0(3)       # load the v4f32 <w0, w1, w2, w3>
	xxmrghw 2, 0, 0   # Produce the following vector <w0, w0, w1, w1>
        xxmrglw 3, 0, 0   # Produce the following vector <w2, w2, w3, w3>
        xvcvspdp 2, 2     # FP-extend to <d0, d1>
	xvcvspdp 3, 3     # FP-extend to <d2, d3>
        stxv 2, 0(5)      # Store <d0, d1> (%vecinit11)
        stxv 3, 0(4)      # Store <d2, d3> (%vecinit4)

[lei]

Ping

[nemanjai]

A few minor nits and a functional problem that needs to be addressed (exiting if the input is not a v4f32).

llvm/lib/Target/PowerPC/PPCISelLowering.cpp

9632 ↗

(On Diff #199674)

Nit: spaces around the ==.

9635 ↗

(On Diff #199674)

I think you mean doubleword here.

9637 ↗

(On Diff #199674)

Nit: the return SDValue() should be on the line under the if.
We also need a check here that the input to the EXTRACT_SUBVECTOR is actually of type v4f32, otherwise this doesn't work.
If you add a test case much like the one you added here, but change all instances of
<4 x float> to <16 x float>. This will not do the right thing.

9638 ↗

(On Diff #199674)

Nit: spaces around :. Also, this is really just Idx >> 1 isn't it?

9641 ↗

(On Diff #199674)

Same nit about the code being on the line under the if. And also, not word, but doubleword.

[lei]

Address review comments.

[nemanjai]

llvm/lib/Target/PowerPC/PPCISelLowering.cpp

9884 ↗

(On Diff #211160)

I don't think this assert does what you want it to.
This makes sure that one of the following is true:

• There are two operands
• The second operand is a constant
• The first operand does not have type v4f32

And what you want is that all of those are true (except the last one needs to be flipped):

assert(Op0.getNumOperands() == 2 &&
       isa<ConstantSDNode(Op0.getOperand(1) &&
       Op0.getOperand(0).getValueType == MVT::v4f32 &&
       "Input must be MVT::v4f32 and operand 2 must be a constant!");

But in any case, I think we should not assert that last one - it can be false without anything being broken (i.e. pre-legalize). This should be an early exit here:

if (Op0.getOperand(0).getValueType() != MVT::v4f32)
  return SDValue();

9895 ↗

(On Diff #211160)

s/double word/doubleword

llvm/test/CodeGen/PowerPC/reduce_scalarization02.ll

11 ↗

(On Diff #211160)

Please add an additional test case where the input type is simply changed from <4 x float> to <16 x float>.

[lei]

Addressed reviewer comments.

[lei]

@nemanjai All comments have been addressed. Please take another look. Thx!

[nemanjai]

Other than the minor nits that you can feel free to fix on the commit, LGTM.

llvm/lib/Target/PowerPC/PPCISelLowering.cpp

9935 ↗

(On Diff #219705)

Sorry, I should have mentioned this before...
It would be clearer that you're simply flipping the bit if you wrote this as
DWord ^= 0x1 or DWord = DWord ^ 0x1

9632 ↗

(On Diff #199674)

Two issues:

• The nit about spaces around == was not addressed
• We don't want the || but an && since we want both two operands and a constant second operand. Then the parens are no longer required either.

llvm/trunk/test/CodeGen/PowerPC/reduce_scalarization02.ll

+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc -verify-machineinstrs -mtriple=powerpc64le-unknown-unknown \
+; RUN:     -mcpu=pwr9 -ppc-asm-full-reg-names \
+; RUN:     -ppc-vsr-nums-as-vr < %s | FileCheck %s
+; RUN: llc -verify-machineinstrs -mtriple=powerpc64-unknown-unknown \
+; RUN:     -mcpu=pwr9 -ppc-asm-full-reg-names \
+; RUN:     -ppc-vsr-nums-as-vr < %s | FileCheck %s --check-prefix=CHECK-BE
+
+; Test reduce scalarization in fpext v2f32 to v2f64 from the extract_subvector v4f32 node.
+
+define dso_local void @test(<4 x float>* nocapture readonly %a, <2 x double>* nocapture %b, <2 x double>* nocapture %c) {
+; CHECK-LABEL: test:
+; CHECK:       # %bb.0: # %entry
+; CHECK-NEXT:    lxv vs0, 0(r3)
+; CHECK-NEXT:    xxmrglw vs1, vs0, vs0
+; CHECK-NEXT:    xxmrghw vs0, vs0, vs0
+; CHECK-NEXT:    xvcvspdp vs1, vs1
+; CHECK-NEXT:    xvcvspdp vs0, vs0
+; CHECK-NEXT:    stxv vs1, 0(r4)
+; CHECK-NEXT:    stxv vs0, 0(r5)
+; CHECK-NEXT:    blr
+;
+; CHECK-BE-LABEL: test:
+; CHECK-BE:       # %bb.0: # %entry
+; CHECK-BE-NEXT:    lxv vs0, 0(r3)
+; CHECK-BE-NEXT:    xxmrghw vs1, vs0, vs0
+; CHECK-BE-NEXT:    xxmrglw vs0, vs0, vs0
+; CHECK-BE-NEXT:    xvcvspdp vs1, vs1
+; CHECK-BE-NEXT:    xvcvspdp vs0, vs0
+; CHECK-BE-NEXT:    stxv vs1, 0(r4)
+; CHECK-BE-NEXT:    stxv vs0, 0(r5)
+; CHECK-BE-NEXT:    blr
+entry:
+  %0 = load <4 x float>, <4 x float>* %a, align 16
+  %shuffle = shufflevector <4 x float> %0, <4 x float> undef, <2 x i32> <i32 0, i32 1>
+  %shuffle1 = shufflevector <4 x float> %0, <4 x float> undef, <2 x i32> <i32 2, i32 3>
+  %vecinit4 = fpext <2 x float> %shuffle to <2 x double>
+  %vecinit11 = fpext <2 x float> %shuffle1 to <2 x double>
+  store <2 x double> %vecinit4, <2 x double>* %b, align 16
+  store <2 x double> %vecinit11, <2 x double>* %c, align 16
+  ret void
+}
+
+; Ensure we don't crash for wider types
+
+define dso_local void @test2(<16 x float>* nocapture readonly %a, <2 x double>* nocapture %b, <2 x double>* nocapture %c) {
+; CHECK-LABEL: test2:
+; CHECK:       # %bb.0: # %entry
+; CHECK-NEXT:    lxv vs0, 0(r3)
+; CHECK-NEXT:    xxsldwi vs1, vs0, vs0, 1
+; CHECK-NEXT:    xscvspdpn f2, vs0
+; CHECK-NEXT:    xxsldwi vs3, vs0, vs0, 3
+; CHECK-NEXT:    xxswapd vs0, vs0
+; CHECK-NEXT:    xscvspdpn f1, vs1
+; CHECK-NEXT:    xscvspdpn f3, vs3
+; CHECK-NEXT:    xscvspdpn f0, vs0
+; CHECK-NEXT:    xxmrghd vs0, vs0, vs3
+; CHECK-NEXT:    xxmrghd vs1, vs2, vs1
+; CHECK-NEXT:    stxv vs0, 0(r4)
+; CHECK-NEXT:    stxv vs1, 0(r5)
+; CHECK-NEXT:    blr
+;
+; CHECK-BE-LABEL: test2:
+; CHECK-BE:       # %bb.0: # %entry
+; CHECK-BE-NEXT:    lxv vs0, 0(r3)
+; CHECK-BE-NEXT:    xxswapd vs1, vs0
+; CHECK-BE-NEXT:    xxsldwi vs2, vs0, vs0, 3
+; CHECK-BE-NEXT:    xscvspdpn f3, vs0
+; CHECK-BE-NEXT:    xxsldwi vs0, vs0, vs0, 1
+; CHECK-BE-NEXT:    xscvspdpn f1, vs1
+; CHECK-BE-NEXT:    xscvspdpn f2, vs2
+; CHECK-BE-NEXT:    xscvspdpn f0, vs0
+; CHECK-BE-NEXT:    xxmrghd vs0, vs3, vs0
+; CHECK-BE-NEXT:    xxmrghd vs1, vs1, vs2
+; CHECK-BE-NEXT:    stxv vs0, 0(r4)
+; CHECK-BE-NEXT:    stxv vs1, 0(r5)
+; CHECK-BE-NEXT:    blr
+entry:
+  %0 = load <16 x float>, <16 x float>* %a, align 16
+  %shuffle = shufflevector <16 x float> %0, <16 x float> undef, <2 x i32> <i32 0, i32 1>
+  %shuffle1 = shufflevector <16 x float> %0, <16 x float> undef, <2 x i32> <i32 2, i32 3>
+  %vecinit4 = fpext <2 x float> %shuffle to <2 x double>
+  %vecinit11 = fpext <2 x float> %shuffle1 to <2 x double>
+  store <2 x double> %vecinit4, <2 x double>* %b, align 16
+  store <2 x double> %vecinit11, <2 x double>* %c, align 16
+  ret void
+}