[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

17671

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 ↗

(On Diff #492800)

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 ↗

(On Diff #492800)

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.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);
     }
 
     if (Subtarget.hasP10Vector()) {
       setOperationAction(ISD::SELECT_CC, MVT::f128, Custom);
     }
   }
 
   if (Subtarget.pairedVectorMemops()) {
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)) {
     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 : 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))),
+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))),
           (v4i32 (VABSDUW (XVNEGSP $A), (XVNEGSP $B)))>;
 } // HasVSX, HasP9Altivec