[PowerPC] Replace PPCISD::VABSD cases with generic ISD::ABDU(X,Y) node

[RKSimon]

Summary

A move towards using the generic ISD::ABDU nodes on more backends

Also support ISD::ABDS for v4i32 types using the existing signbit flip trick

PowerPC has a select(icmp_ugt(x,y),sub(x,y),sub(y,x)) -> abdu(x,y) that I intend to move to DAGCombiner in a future patch.

I don't think the PPCISD::VABSD(X,Y,1) v4i32 combine was legal (at least alive2 didn't think so) - so I've removed it.

[nemanjai]

Thanks for updating this. I am just curious, do the combines you removed just end up getting combined to a VSELECT?

[RKSimon]

In D142313#4075615, @nemanjai wrote:

Thanks for updating this. I am just curious, do the combines you removed just end up getting combined to a VSELECT?

@dmgreen added them to DAGCombiner as part of adding the generic abds/abdu nodes

What should be done with the PPCISD::VABSD nodes? They can only be used for the PPCISD::VABSD(X, 1) case now - should they be simplified/renamed?

[RKSimon]

llvm/lib/Target/PowerPC/PPCISelLowering.cpp

17650–17651

I'm wondering if this is actually correct? If there had been SIGN_EXTEND/SIGN_EXTEND_VECTOR_INREG equivalent nodes to the previous ABDU pattern then this would be ABSD.

https://alive2.llvm.org/ce/z/k8wpeF

[RKSimon]

Adds support for ISD::ABDS for v4i32 types using the existing signbit flip trick and remove the PPCISD::VABSD node type entirely

I've tentatively removed the "(abs (sub a, b) --> (vabsd a b 1))" fold as afaict its not correct: https://alive2.llvm.org/ce/z/jc2hLU

[RKSimon]

ping?

[RKSimon]

ping2?

llvm/test/CodeGen/PowerPC/ppc64-P9-vabsd.ll

1249

if we replace this with sub nsw then the fold is legal

[nemanjai]

LGTM. Thanks for cleaning this up and sorry about the delay in reviewing it.

llvm/test/CodeGen/PowerPC/ppc64-P9-vabsd.ll

1249

I suppose the test case then becomes equivalent to sub_absv_32() other than the use of the vmaxsw intrinsic. I think we can just add the flag so the test case shows the desired code-gen.

[lkail]

Hi @RKSimon

The ABS(SUB(X,Y)) -> PPCISD::VABSD(X,Y,1) v4i32 combine wasn't legal (https://alive2.llvm.org/ce/z/jc2hLU) - so I've removed it, having already added the legal sub nsw tests equivalent.

For case where X and Y are both zero extended from other values, the original transformation looks still correct. See https://alive2.llvm.org/ce/z/LWaEEg.

llvm/lib/Target/PowerPC/PPCISelLowering.h

enum NodeType : unsigned {
     /// sequence replaces a load and needs to provide the same number
     /// of outputs.
     XXSWAPD,
 
     /// An SDNode for swaps that are not associated with any loads/stores
     /// and thereby have no chain.
     SWAP_NO_CHAIN,
 
-    /// An SDNode for Power9 vector absolute value difference.
-    /// operand #0 vector
-    /// operand #1 vector
-    /// operand #2 constant i32 0 or 1, to indicate whether needs to patch
-    /// the most significant bit for signed i32
-    ///
-    /// Power9 VABSD* instructions are designed to support unsigned integer
-    /// vectors (byte/halfword/word), if we want to make use of them for signed
-    /// integer vectors, we have to flip their sign bits first. To flip sign bit
-    /// for byte/halfword integer vector would become inefficient, but for word
-    /// integer vector, we can leverage XVNEGSP to make it efficiently. eg:
-    /// abs(sub(a,b)) => VABSDUW(a+0x80000000, b+0x80000000)
-    ///               => VABSDUW((XVNEGSP a), (XVNEGSP b))
-    VABSD,
-
     /// FP_EXTEND_HALF(VECTOR, IDX) - Custom extend upper (IDX=0) half or
     /// lower (IDX=1) half of v4f32 to v2f64.
     FP_EXTEND_HALF,
 
     /// MAT_PCREL_ADDR = Materialize a PC Relative address. This can be done
     /// either through an add like PADDI or through a PC Relative load like
     /// PLD.
     MAT_PCREL_ADDR,

llvm/lib/Target/PowerPC/PPCISelLowering.cpp

if (Subtarget.hasP9Altivec()) {
       }
       setOperationAction(ISD::SIGN_EXTEND_INREG, MVT::v4i8,  Legal);
       setOperationAction(ISD::SIGN_EXTEND_INREG, MVT::v4i16, Legal);
       setOperationAction(ISD::SIGN_EXTEND_INREG, MVT::v4i32, Legal);
       setOperationAction(ISD::SIGN_EXTEND_INREG, MVT::v2i8,  Legal);
       setOperationAction(ISD::SIGN_EXTEND_INREG, MVT::v2i16, Legal);
       setOperationAction(ISD::SIGN_EXTEND_INREG, MVT::v2i32, Legal);
       setOperationAction(ISD::SIGN_EXTEND_INREG, MVT::v2i64, Legal);
+
+      setOperationAction(ISD::ABDU, MVT::v16i8, Legal);
+      setOperationAction(ISD::ABDU, MVT::v8i16, Legal);
+      setOperationAction(ISD::ABDU, MVT::v4i32, Legal);
+      setOperationAction(ISD::ABDS, MVT::v4i32, Legal);
     }
 
     if (Subtarget.hasP10Vector()) {
       setOperationAction(ISD::SELECT_CC, MVT::f128, Custom);
     }
   }
 
   if (Subtarget.pairedVectorMemops()) {
case PPCISD::PADDI_DTPREL:
     return "PPCISD::PADDI_DTPREL";
   case PPCISD::VADD_SPLAT:      return "PPCISD::VADD_SPLAT";
   case PPCISD::SC:              return "PPCISD::SC";
   case PPCISD::CLRBHRB:         return "PPCISD::CLRBHRB";
   case PPCISD::MFBHRBE:         return "PPCISD::MFBHRBE";
   case PPCISD::RFEBB:           return "PPCISD::RFEBB";
   case PPCISD::XXSWAPD:         return "PPCISD::XXSWAPD";
   case PPCISD::SWAP_NO_CHAIN:   return "PPCISD::SWAP_NO_CHAIN";
-  case PPCISD::VABSD:           return "PPCISD::VABSD";
   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_HALF:  return "PPCISD::FP_EXTEND_HALF";
   case PPCISD::MAT_PCREL_ADDR:  return "PPCISD::MAT_PCREL_ADDR";
   case PPCISD::TLS_DYNAMIC_MAT_PCREL_ADDR:
if (LHS.getOpcode() == ISD::INTRINSIC_WO_CHAIN &&
                          DAG.getConstant(CompOpc, dl, MVT::i32),
                          DAG.getRegister(PPC::CR6, MVT::i32),
                          N->getOperand(4), CompNode.getValue(1));
     }
     break;
   }
   case ISD::BUILD_VECTOR:
     return DAGCombineBuildVector(N, DCI);
-  case ISD::ABS:
-    return combineABS(N, DCI);
   case ISD::VSELECT:
     return combineVSelect(N, DCI);
   }
 
   return SDValue();
 }
 
 SDValue
if (Subtarget.useCRBits()) {
     // Check if CRBits did anything and return that if it did.
     if (SDValue CRTruncValue = DAGCombineTruncBoolExt(N, DCI))
       return CRTruncValue;
   }
 
   SDLoc dl(N);
   SDValue Op0 = N->getOperand(0);
 
-  // fold (truncate (abs (sub (zext a), (zext b)))) -> (vabsd a, b)
-  if (Subtarget.hasP9Altivec() && Op0.getOpcode() == ISD::ABS) {
-    EVT VT = N->getValueType(0);
-    if (VT != MVT::v4i32 && VT != MVT::v8i16 && VT != MVT::v16i8)
-      return SDValue();
-    SDValue Sub = Op0.getOperand(0);
-    if (Sub.getOpcode() == ISD::SUB) {
-      SDValue SubOp0 = Sub.getOperand(0);
-      SDValue SubOp1 = Sub.getOperand(1);
-      if ((SubOp0.getOpcode() == ISD::ZERO_EXTEND) &&
-          (SubOp1.getOpcode() == ISD::ZERO_EXTEND)) {
-        return DCI.DAG.getNode(PPCISD::VABSD, dl, VT, SubOp0.getOperand(0),
-                               SubOp1.getOperand(0),
-                               DCI.DAG.getTargetConstant(0, dl, MVT::i32));
-      }
-    }
-  }
-
   // Looking for a truncate of i128 to i64.
   if (Op0.getValueType() != MVT::i128 || N->getValueType(0) != MVT::i64)
     return SDValue();
 
   int EltToExtract = DCI.DAG.getDataLayout().isBigEndian() ? 1 : 0;
 
   // SRL feeding TRUNCATE.
   if (Op0.getOpcode() == ISD::SRL) {
if (const ConstantInt *CI = dyn_cast<ConstantInt>(Mask)) {
     int64_t ConstVal = CI->getZExtValue();
     return isUInt<16>(ConstVal) ||
       (isUInt<16>(ConstVal >> 16) && !(ConstVal & 0xFFFF));
   }
 
   // For non-constant masks, we can always use the record-form and.
   return true;
 }
 
-// Transform (abs (sub (zext a), (zext b))) to (vabsd a b 0)
-// Transform (abs (sub (zext a), (zext_invec b))) to (vabsd a b 0)
-// Transform (abs (sub (zext_invec a), (zext_invec b))) to (vabsd a b 0)
-// Transform (abs (sub (zext_invec a), (zext b))) to (vabsd a b 0)
-// Transform (abs (sub a, b) to (vabsd a b 1)) if a & b of type v4i32
-SDValue PPCTargetLowering::combineABS(SDNode *N, DAGCombinerInfo &DCI) const {
-  assert((N->getOpcode() == ISD::ABS) && "Need ABS node here");
-  assert(Subtarget.hasP9Altivec() &&
-         "Only combine this when P9 altivec supported!");
-  EVT VT = N->getValueType(0);
-  if (VT != MVT::v4i32 && VT != MVT::v8i16 && VT != MVT::v16i8)
-    return SDValue();
-
-  SelectionDAG &DAG = DCI.DAG;
-  SDLoc dl(N);
-  if (N->getOperand(0).getOpcode() == ISD::SUB) {
-    // Even for signed integers, if it's known to be positive (as signed
-    // integer) due to zero-extended inputs.
-    unsigned SubOpcd0 = N->getOperand(0)->getOperand(0).getOpcode();
-    unsigned SubOpcd1 = N->getOperand(0)->getOperand(1).getOpcode();
-    if ((SubOpcd0 == ISD::ZERO_EXTEND ||
-         SubOpcd0 == ISD::ZERO_EXTEND_VECTOR_INREG) &&
-        (SubOpcd1 == ISD::ZERO_EXTEND ||
-         SubOpcd1 == ISD::ZERO_EXTEND_VECTOR_INREG)) {
-      return DAG.getNode(PPCISD::VABSD, dl, N->getOperand(0).getValueType(),
-                         N->getOperand(0)->getOperand(0),
-                         N->getOperand(0)->getOperand(1),
-                         DAG.getTargetConstant(0, dl, MVT::i32));
-    }
-
-    // For type v4i32, it can be optimized with xvnegsp + vabsduw
-    if (N->getOperand(0).getValueType() == MVT::v4i32 &&
-        N->getOperand(0).hasOneUse()) {
-      return DAG.getNode(PPCISD::VABSD, dl, N->getOperand(0).getValueType(),
-                         N->getOperand(0)->getOperand(0),
-                         N->getOperand(0)->getOperand(1),
-                         DAG.getTargetConstant(1, dl, MVT::i32));
-    }
-  }
-
-  return SDValue();
-}
-
 // For type v4i32/v8ii16/v16i8, transform
-// from (vselect (setcc a, b, setugt), (sub a, b), (sub b, a)) to (vabsd a, b)
-// from (vselect (setcc a, b, setuge), (sub a, b), (sub b, a)) to (vabsd a, b)
-// from (vselect (setcc a, b, setult), (sub b, a), (sub a, b)) to (vabsd a, b)
-// from (vselect (setcc a, b, setule), (sub b, a), (sub a, b)) to (vabsd a, b)
+// from (vselect (setcc a, b, setugt), (sub a, b), (sub b, a)) to (abdu a, b)
+// from (vselect (setcc a, b, setuge), (sub a, b), (sub b, a)) to (abdu a, b)
+// from (vselect (setcc a, b, setult), (sub b, a), (sub a, b)) to (abdu a, b)
+// from (vselect (setcc a, b, setule), (sub b, a), (sub a, b)) to (abdu a, b)
+// TODO: Move this to DAGCombiner?
 SDValue PPCTargetLowering::combineVSelect(SDNode *N,
                                           DAGCombinerInfo &DCI) const {
   assert((N->getOpcode() == ISD::VSELECT) && "Need VSELECT node here");
   assert(Subtarget.hasP9Altivec() &&
          "Only combine this when P9 altivec supported!");
 
   SelectionDAG &DAG = DCI.DAG;
   SDLoc dl(N);
SDValue PPCTargetLowering::combineVSelect(SDNode *N,
 
   // SETCC CmpOpnd1 CmpOpnd2 cond
   // TrueOpnd = CmpOpnd1 - CmpOpnd2
   // FalseOpnd = CmpOpnd2 - CmpOpnd1
   if (TrueOpnd.getOperand(0) == CmpOpnd1 &&
       TrueOpnd.getOperand(1) == CmpOpnd2 &&
       FalseOpnd.getOperand(0) == CmpOpnd2 &&
       FalseOpnd.getOperand(1) == CmpOpnd1) {
-    return DAG.getNode(PPCISD::VABSD, dl, N->getOperand(1).getValueType(),
-                       CmpOpnd1, CmpOpnd2,
-                       DAG.getTargetConstant(0, dl, MVT::i32));
+    return DAG.getNode(ISD::ABDU, dl, N->getOperand(1).getValueType(), CmpOpnd1,
+                       CmpOpnd2, DAG.getTargetConstant(0, dl, MVT::i32));
   }
 
   return SDValue();
 }
 
 /// getAddrModeForFlags - Based on the set of address flags, select the most
 /// optimal instruction format to match by.
 PPC::AddrMode PPCTargetLowering::getAddrModeForFlags(unsigned Flags) const {

llvm/lib/Target/PowerPC/PPCInstrVSX.td

def SDT_PPCstxvd2x : SDTypeProfile<0, 2, [
   SDTCisVT<0, v2f64>, SDTCisPtrTy<1>
 ]>;
 def SDT_PPCxxswapd : SDTypeProfile<1, 1, [
   SDTCisSameAs<0, 1>
 ]>;
 def SDTVecConv : SDTypeProfile<1, 2, [
   SDTCisVec<0>, SDTCisVec<1>, SDTCisPtrTy<2>
 ]>;
-def SDTVabsd : SDTypeProfile<1, 3, [
-  SDTCisVec<0>, SDTCisSameAs<0, 1>, SDTCisSameAs<0, 2>, SDTCisVT<3, i32>
-]>;
 def SDT_PPCld_vec_be : SDTypeProfile<1, 1, [
   SDTCisVec<0>, SDTCisPtrTy<1>
 ]>;
 def SDT_PPCst_vec_be : SDTypeProfile<0, 2, [
   SDTCisVec<0>, SDTCisPtrTy<1>
 ]>;
 
 def SDT_PPCxxperm : SDTypeProfile<1, 3, [
def PPCst_vec_be : SDNode<"PPCISD::STORE_VEC_BE", SDT_PPCst_vec_be,
                         [SDNPHasChain, SDNPMayStore]>;
 def PPCxxswapd : SDNode<"PPCISD::XXSWAPD", SDT_PPCxxswapd, [SDNPHasChain]>;
 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 PPCfpexth : SDNode<"PPCISD::FP_EXTEND_HALF", SDT_PPCfpexth, []>;
 def PPCldvsxlh : SDNode<"PPCISD::LD_VSX_LH", SDT_PPCldvsxlh,
                         [SDNPHasChain, SDNPMayLoad, SDNPMemOperand]>;
 def PPCldsplat : SDNode<"PPCISD::LD_SPLAT", SDT_PPCldsplat,
                         [SDNPHasChain, SDNPMayLoad, SDNPMemOperand]>;
 def PPCzextldsplat : SDNode<"PPCISD::ZEXT_LD_SPLAT", SDT_PPCldsplat,
                         [SDNPHasChain, SDNPMayLoad, SDNPMemOperand]>;

 // Unsiged int in vsx register -> QP
 def : Pat<(f128 (uint_to_fp (i32 (PPCmfvsr f64:$src)))),
           (f128 (XSCVUDQP
                   (XXEXTRACTUW (SUBREG_TO_REG (i64 1), $src, sub_64), 8)))>;
 } // HasVSX, HasP9Vector, IsLittleEndian
 
 // Any Power9 VSX subtarget that supports Power9 Altivec.
 let Predicates = [HasVSX, HasP9Altivec] in {
-// Put this P9Altivec related definition here since it's possible to be
-// selected to VSX instruction xvnegsp, avoid possible undef.
-def : Pat<(v4i32 (PPCvabsd v4i32:$A, v4i32:$B, (i32 0))),
+// Unsigned absolute-difference.
+def : Pat<(v4i32 (abdu v4i32:$A, v4i32:$B)),
           (v4i32 (VABSDUW $A, $B))>;
 
-def : Pat<(v8i16 (PPCvabsd v8i16:$A, v8i16:$B, (i32 0))),
+def : Pat<(v8i16 (abdu v8i16:$A, v8i16:$B)),
           (v8i16 (VABSDUH $A, $B))>;
 
-def : Pat<(v16i8 (PPCvabsd v16i8:$A, v16i8:$B, (i32 0))),
+def : Pat<(v16i8 (abdu v16i8:$A, v16i8:$B)),
           (v16i8 (VABSDUB $A, $B))>;
 
-// As PPCVABSD description, the last operand indicates whether do the
-// sign bit flip.
-def : Pat<(v4i32 (PPCvabsd v4i32:$A, v4i32:$B, (i32 1))),
+// Signed absolute-difference.
+// Power9 VABSD* instructions are designed to support unsigned integer
+// vectors (byte/halfword/word), if we want to make use of them for signed
+// integer vectors, we have to flip their sign bits first. To flip sign bit
+// for byte/halfword integer vector would become inefficient, but for word
+// integer vector, we can leverage XVNEGSP to make it efficiently.
+def : Pat<(v4i32 (abds v4i32:$A, v4i32:$B)),
           (v4i32 (VABSDUW (XVNEGSP $A), (XVNEGSP $B)))>;
 } // HasVSX, HasP9Altivec
 
 // Big endian Power9 64Bit VSX subtargets with P9 Altivec support.
 let Predicates = [HasVSX, HasP9Altivec, IsBigEndian, IsPPC64] in {
 def : Pat<(i64 (anyext (i32 (vector_extract v16i8:$S, i64:$Idx)))),
           (VEXTUBLX $Idx, $S)>;
 

llvm/test/CodeGen/PowerPC/ppc64-P9-vabsd.ll

entry:
   %conv122 = trunc i32 %15 to i8
   %vecins123 = insertelement <16 x i8> %vecins115, i8 %conv122, i32 15
   ret <16 x i8> %vecins123
 }
 
 define <4 x i32> @sub_absv_vec_32(<4 x i32> %a, <4 x i32> %b) local_unnamed_addr {
 ; CHECK-PWR9-LABEL: sub_absv_vec_32:
 ; CHECK-PWR9:       # %bb.0: # %entry
-; CHECK-PWR9-NEXT:    xvnegsp v3, v3
-; CHECK-PWR9-NEXT:    xvnegsp v2, v2
-; CHECK-PWR9-NEXT:    vabsduw v2, v2, v3
+; CHECK-PWR9-NEXT:    vsubuwm v2, v2, v3
+; CHECK-PWR9-NEXT:    vnegw v3, v2
+; CHECK-PWR9-NEXT:    vmaxsw v2, v2, v3
 ; CHECK-PWR9-NEXT:    blr
 ;
 ; CHECK-PWR78-LABEL: sub_absv_vec_32:
 ; CHECK-PWR78:       # %bb.0: # %entry
 ; CHECK-PWR78-NEXT:    xxlxor v4, v4, v4
 ; CHECK-PWR78-NEXT:    vsubuwm v2, v2, v3
 ; CHECK-PWR78-NEXT:    vsubuwm v3, v4, v2
 ; CHECK-PWR78-NEXT:    vmaxsw v2, v2, v3
 ; CHECK-PWR78-NEXT:    blr
 entry:
   %sub = sub <4 x i32> %a, %b
   %sub.i = sub <4 x i32> zeroinitializer, %sub
   %0 = tail call <4 x i32> @llvm.ppc.altivec.vmaxsw(<4 x i32> %sub, <4 x i32> %sub.i)
   ret <4 x i32> %0
 }
 
 define <8 x i16> @sub_absv_vec_16(<8 x i16> %a, <8 x i16> %b) local_unnamed_addr {
 ; CHECK-PWR9-LABEL: sub_absv_vec_16:
 ; CHECK-PWR9:       # %bb.0: # %entry