[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

9918

Nit: spaces around the ==.

9921

I think you mean doubleword here.

9923

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.

9924

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

9927

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

9918

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();

9929

s/double word/doubleword

llvm/test/CodeGen/PowerPC/reduce_scalarization02.ll

12

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

9918

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.

9935

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

llvm/lib/Target/PowerPC/PPCISelLowering.h

enum NodeType : unsigned {
 
       /// QVESPLATI = This corresponds to the QPX qvesplati instruction.
       QVESPLATI,
 
       /// QBFLT = Access the underlying QPX floating-point boolean
       /// representation.
       QBFLT,
 
-      /// Custom extend v4f32 to v2f64.
-      FP_EXTEND_LH,
+      /// FP_EXTEND_HALF(VECTOR, IDX) - Custom extend upper (IDX=0) half or
+      /// lower (IDX=1) half of v4f32 to v2f64.
+      FP_EXTEND_HALF,
 
       /// CHAIN = STBRX CHAIN, GPRC, Ptr, Type - This is a
       /// byte-swapping store instruction.  It byte-swaps the low "Type" bits of
       /// the GPRC input, then stores it through Ptr.  Type can be either i16 or
       /// i32.
       STBRX = ISD::FIRST_TARGET_MEMORY_OPCODE,
 
       /// GPRC, CHAIN = LBRX CHAIN, Ptr, Type - This is a

llvm/lib/Target/PowerPC/PPCISelLowering.cpp

const char *PPCTargetLowering::getTargetNodeName(unsigned Opcode) const {
   case PPCISD::QVESPLATI:       return "PPCISD::QVESPLATI";
   case PPCISD::QBFLT:           return "PPCISD::QBFLT";
   case PPCISD::QVLFSb:          return "PPCISD::QVLFSb";
   case PPCISD::BUILD_FP128:     return "PPCISD::BUILD_FP128";
   case PPCISD::BUILD_SPE64:     return "PPCISD::BUILD_SPE64";
   case PPCISD::EXTRACT_SPE:     return "PPCISD::EXTRACT_SPE";
   case PPCISD::EXTSWSLI:        return "PPCISD::EXTSWSLI";
   case PPCISD::LD_VSX_LH:       return "PPCISD::LD_VSX_LH";
-  case PPCISD::FP_EXTEND_LH:    return "PPCISD::FP_EXTEND_LH";
+  case PPCISD::FP_EXTEND_HALF:  return "PPCISD::FP_EXTEND_HALF";
   }
   return nullptr;
 }
 
 EVT PPCTargetLowering::getSetCCResultType(const DataLayout &DL, LLVMContext &C,
                                           EVT VT) const {
   if (!VT.isVector())
     return Subtarget.useCRBits() ? MVT::i1 : MVT::i32;
if (Op.getValueType() != MVT::v2f64 ||
     return SDValue();
 
   SDLoc dl(Op);
   SDValue Op0 = Op.getOperand(0);
 
   switch (Op0.getOpcode()) {
   default:
     return SDValue();
+  case ISD::EXTRACT_SUBVECTOR: {
+    assert((Op0.getNumOperands()==2 || isa<ConstantSDNode>(Op0->getOperand(1)))
+     && "Node should have 2 operands with second one being a constant!");
+
+    if (Op0.getOperand(0).getValueType() != MVT::v4f32)
+        return SDValue();
+
+    // Custom lower is only done for high or low doubleword.
+    int Idx = cast<ConstantSDNode>(Op0.getOperand(1))->getZExtValue();
+    if (Idx % 2 != 0)
+      return SDValue();
+
+    // Since input is v4f32, at this point Idx is either 0 or 2.
+    // Shift to get the doubleword position we want.
+    int DWord = Idx >> 1;
+
+    // High and low word positions are different on little endian.
+    if (Subtarget.isLittleEndian())
+      DWord = !DWord;
+
+    return DAG.getNode(PPCISD::FP_EXTEND_HALF, dl, MVT::v2f64,
+                       Op0.getOperand(0), DAG.getConstant(DWord, dl, MVT::i32));
+  }
   case ISD::FADD:
   case ISD::FMUL:
   case ISD::FSUB: {
     SDValue NewLoad[2];
     for (unsigned i = 0, ie = Op0.getNumOperands(); i != ie; ++i) {
       // Ensure both input are loads.
       SDValue LdOp = Op0.getOperand(i);
       if (LdOp.getOpcode() != ISD::LOAD)
         return SDValue();
       // Generate new load node.
       LoadSDNode *LD = cast<LoadSDNode>(LdOp);
       SDValue LoadOps[] = { LD->getChain(), LD->getBasePtr() };
       NewLoad[i] =
         DAG.getMemIntrinsicNode(PPCISD::LD_VSX_LH, dl,
                                 DAG.getVTList(MVT::v4f32, MVT::Other),
                                 LoadOps, LD->getMemoryVT(),
                                 LD->getMemOperand());
     }
     SDValue NewOp = DAG.getNode(Op0.getOpcode(), SDLoc(Op0), MVT::v4f32,
                               NewLoad[0], NewLoad[1],
                               Op0.getNode()->getFlags());
-    return DAG.getNode(PPCISD::FP_EXTEND_LH, dl, MVT::v2f64, NewOp);
+    return DAG.getNode(PPCISD::FP_EXTEND_HALF, dl, MVT::v2f64, NewOp,
+                       DAG.getConstant(0, dl, MVT::i32));
   }
   case ISD::LOAD: {
     LoadSDNode *LD = cast<LoadSDNode>(Op0);
     SDValue LoadOps[] = { LD->getChain(), LD->getBasePtr() };
     SDValue NewLd =
       DAG.getMemIntrinsicNode(PPCISD::LD_VSX_LH, dl,
                               DAG.getVTList(MVT::v4f32, MVT::Other),
                               LoadOps, LD->getMemoryVT(), LD->getMemOperand());
-    return DAG.getNode(PPCISD::FP_EXTEND_LH, dl, MVT::v2f64, NewLd);
+    return DAG.getNode(PPCISD::FP_EXTEND_HALF, dl, MVT::v2f64, NewLd,
+                       DAG.getConstant(0, dl, MVT::i32));
   }
   }
   llvm_unreachable("ERROR:Should return for all cases within swtich.");
 }
 
 /// LowerOperation - Provide custom lowering hooks for some operations.
 ///
 SDValue PPCTargetLowering::LowerOperation(SDValue Op, SelectionDAG &DAG) const {

llvm/lib/Target/PowerPC/PPCInstrVSX.td

 def spilltovsrrc : RegisterOperand<SPILLTOVSRRC> {
   let ParserMatchClass = PPCRegSPILLTOVSRRCAsmOperand;
 }
 
 def SDT_PPCldvsxlh : SDTypeProfile<1, 1, [
   SDTCisVT<0, v4f32>, SDTCisPtrTy<1>
 ]>;
 
-def SDT_PPCfpextlh : SDTypeProfile<1, 1, [
-  SDTCisVT<0, v2f64>, SDTCisVT<1, v4f32>
+def SDT_PPCfpexth : SDTypeProfile<1, 2, [
+  SDTCisVT<0, v2f64>, SDTCisVT<1, v4f32>, SDTCisPtrTy<2>
 ]>;
 
 // Little-endian-specific nodes.
 def SDT_PPClxvd2x : SDTypeProfile<1, 1, [
   SDTCisVT<0, v2f64>, SDTCisPtrTy<1>
 ]>;
 def SDT_PPCstxvd2x : SDTypeProfile<0, 2, [
   SDTCisVT<0, v2f64>, SDTCisPtrTy<1>

 def PPCmfvsr : SDNode<"PPCISD::MFVSR", SDTUnaryOp, []>;
 def PPCmtvsra : SDNode<"PPCISD::MTVSRA", SDTUnaryOp, []>;
 def PPCmtvsrz : SDNode<"PPCISD::MTVSRZ", SDTUnaryOp, []>;
 def PPCsvec2fp : SDNode<"PPCISD::SINT_VEC_TO_FP", SDTVecConv, []>;
 def PPCuvec2fp: SDNode<"PPCISD::UINT_VEC_TO_FP", SDTVecConv, []>;
 def PPCswapNoChain : SDNode<"PPCISD::SWAP_NO_CHAIN", SDT_PPCxxswapd>;
 def PPCvabsd : SDNode<"PPCISD::VABSD", SDTVabsd, []>;
 
-def PPCfpextlh : SDNode<"PPCISD::FP_EXTEND_LH", SDT_PPCfpextlh, []>;
+def PPCfpexth : SDNode<"PPCISD::FP_EXTEND_HALF", SDT_PPCfpexth, []>;
 def PPCldvsxlh : SDNode<"PPCISD::LD_VSX_LH", SDT_PPCldvsxlh,
                         [SDNPHasChain, SDNPMayLoad, SDNPMemOperand]>;
 
 multiclass XX3Form_Rcr<bits<6> opcode, bits<7> xo, string asmbase,
                     string asmstr, InstrItinClass itin, Intrinsic Int,
                     ValueType OutTy, ValueType InTy> {
   let BaseName = asmbase in {
     def NAME : XX3Form_Rc<opcode, xo, (outs vsrc:$XT), (ins vsrc:$XA, vsrc:$XB),

 def : Pat<(v2f64 (PPCsvec2fp v4i32:$C, 1)),
           (v2f64 (XVCVSXWDP (v2i64 (XXMRGLW $C, $C))))>;
 
 def : Pat<(v2f64 (PPCuvec2fp v4i32:$C, 0)),
           (v2f64 (XVCVUXWDP (v2i64 (XXMRGHW $C, $C))))>;
 def : Pat<(v2f64 (PPCuvec2fp v4i32:$C, 1)),
           (v2f64 (XVCVUXWDP (v2i64 (XXMRGLW $C, $C))))>;
 
-def : Pat<(v2f64 (PPCfpextlh v4f32:$C)), (XVCVSPDP (XXMRGHW $C, $C))>;
+def : Pat<(v2f64 (PPCfpexth v4f32:$C, 0)), (XVCVSPDP (XXMRGHW $C, $C))>;
+def : Pat<(v2f64 (PPCfpexth v4f32:$C, 1)), (XVCVSPDP (XXMRGLW $C, $C))>;
 
 // Loads.
 let Predicates = [HasVSX, HasOnlySwappingMemOps] in {
   def : Pat<(v2f64 (PPClxvd2x xoaddr:$src)), (LXVD2X xoaddr:$src)>;
 
   // Stores.
   def : Pat<(int_ppc_vsx_stxvd2x v2f64:$rS, xoaddr:$dst),
             (STXVD2X $rS, xoaddr:$dst)>;

llvm/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
+}