[PowerPC] Add custom lowering for SELECT_CC fp128 using xsmaxcqp

[tingwang]

Summary

Power ISA 3.1 adds xsmaxcqp/xsmincqp for quad-precision type-c max/min selection, and this opens the opportunity to improve instruction selection on: llvm.maxnum.f128, llvm.minnum.f128, and select_cc ordered gt/lt and (don't care) gt/lt.

[qiucf]

llvm/lib/Target/PowerPC/PPCISelLowering.cpp

1345

We already have a setOperationAction(ISD::SELECT_CC, MVT::f128, Custom); under Subtarget.hasVSX(), so don't need this.

8028

We don't need to add new ISD node, but XS(MAX|MIN)CDP node need renaming.

llvm/lib/Target/PowerPC/PPCInstrInfo.td

223

Operator PPCxsmaxc is type agnostic, we don't need to define PPCxsmaxcq.

llvm/lib/Target/PowerPC/PPCInstrPrefix.td

1915–1918

I'm not sure why xscmpeqqp are located here, but this should be better placed at PPCInstrVSX.td.

llvm/test/CodeGen/PowerPC/scalar-min-max-p10.ll

2

Why not add them into scalar-min-max.ll (with a new -mcpu=pwr10 runline)?

[tingwang]

llvm/lib/Target/PowerPC/PPCISelLowering.cpp

1345

That setOperationAction is located under the else part of Subtarget.hasP9Vector(), since P9Vector is part of P9InheritableFeatures, P10 test Subtarget.hasP9Vector() as true, so not hit that setOperationAction. Since this is supported by P10, and do not see Subtarget.hasP10Vector() predicate in this file, so added one under the Subtarget.hasAltivec() big block.

llvm/lib/Target/PowerPC/PPCInstrPrefix.td

1915–1918

Thanks! I will move XS(MAX|MIN)CQP to sit together with XSMAXCDP.

llvm/test/CodeGen/PowerPC/scalar-min-max-p10.ll

2

That test checks pwr8 behavior, which I don't like to do for fp128, and that's why added a separate file.

[tingwang]

llvm/lib/Target/PowerPC/PPCInstrPrefix.td

1915–1918

A closer look at PPCInstrPrefix.td and PPCInstrVSX.td, all instructions with Power10Instrs predicate are located inside PPCInstrPrefix.td, and does not have any in PPCInstrVSX.td. There maybe some other preference regarding the placement, guess just leave it as is...

[tingwang]

Renamed PPCISD::XS[MAX|MIN]CDP to PPCISD::XS[MAX|MIN]C, and use the nodes for both dual-precision and quad-precision type-c instructions

[qiucf]

llvm/lib/Target/PowerPC/PPCInstrPrefix.td

1915–1918

The name 'Prefix' stands for new ISA 3.1 feature - prefixed instruction, but 128-bit fp max/min is not one of them. Anyway, it's okay to leave them here and we'll do clean-up in the future when all Power10 instructions are implemented.

[shchenz]

llvm/lib/Target/PowerPC/PPCISelLowering.cpp

1345

What if we set Custom for SELECT_CC for all targets hasVSX()? I don't see a reason why we can not set setOperationAction(ISD::SELECT_CC, MVT::f128, Custom); on Power9. If so, we can just set SELECT_CC for type f128 under hasVSX() instead of setting it two times for P10 and hasVSX()?

[tingwang]

llvm/lib/Target/PowerPC/PPCISelLowering.cpp

1345

Had a try doing Custom on Power9 to SELECT_CC, that path caused dump. Following is the path taken by fp128/Power9/SELECT_CC, and picks up PPC::XSCMPUQP:
PPCDAGToDAGISel::Select
-> case ISD::SELECT_CC
--> SelectCC()

Seems the expected code paths for Power9 and Power10 are different, then we still need a predicate to differentiate that?

[shchenz]

llvm/lib/Target/PowerPC/PPCISelLowering.cpp

1345

OK, thanks for the explanation. I think it crashes because we don't handle PPCISD::XSMAXC and PPCISD::XSMINC for type f128 on Power9 in the legalizer.

However, I really think the logic for select_cc is kind of confusing on Power, we legalize it(set operation as custom) for Power8 and Power10, and we set it legal on Power9 and then do the selecting just before we select from td files. Maybe we can do some code refactor here later to make the logic be more clear. This should be out of this PR.

llvm/test/CodeGen/PowerPC/scalar-min-max-p10.ll

1

Better to generate check lines by using scripts like ./utils/update_llc_test_checks.py.

And could you please also add a RUN line for AIX?

[nemanjai]

llvm/lib/Target/PowerPC/PPCInstrPrefix.td

1915–1918

Please do not move any instructions out of this file piecemeal at this time. The name of this file is temporary as we kept all the Power10 instructions in one file during the incremental development process prior to the announcement of Power10.

We need to decide where we want the various groups of instructions to live and move them and/or rename files accordingly. This will be done all at once to make it obvious in the repo history when this happened and what was happening.

[tingwang]

llvm/lib/Target/PowerPC/PPCInstrPrefix.td

1915–1918

Thank you for the input. Will leave them here.

llvm/test/CodeGen/PowerPC/scalar-min-max-p10.ll

1

Thanks for the comments, will update the test case using the script.

[tingwang]

(1) Add llvm.maxnum/minnum.f128 check case for P10
(2) Update test case using the script: update_llc_test_checks.py

[shchenz]

LGTM. Please hold on some days for other reviewers. Thanks for adding this support.

[nemanjai]

LGTM

[amyk]

Overall LGTM.

llvm/lib/Target/PowerPC/PPCInstrPrefix.td

1915–1918

These are meant to be put under [IsISA3_1, HasVSX], right?

llvm/lib/Target/PowerPC/PPCISelLowering.h

namespace llvm {
   enum NodeType : unsigned {
     // Start the numbering where the builtin ops and target ops leave off.
     FIRST_NUMBER = ISD::BUILTIN_OP_END,
 
     /// FSEL - Traditional three-operand fsel node.
     ///
     FSEL,
 
-    /// XSMAXCDP, XSMINCDP - C-type min/max instructions.
-    XSMAXCDP,
-    XSMINCDP,
+    /// XSMAXC[DQ]P, XSMINC[DQ]P - C-type min/max instructions.
+    XSMAXC,
+    XSMINC,
 
     /// FCFID - The FCFID instruction, taking an f64 operand and producing
     /// and f64 value containing the FP representation of the integer that
     /// was temporarily in the f64 operand.
     FCFID,
 
     /// Newer FCFID[US] integer-to-floating-point conversion instructions for
     /// unsigned integers and single-precision outputs.

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);
     }
+
+    if (Subtarget.hasP10Vector()) {
+      setOperationAction(ISD::SELECT_CC, MVT::f128, Custom);
+    }
   }
 
   if (Subtarget.pairedVectorMemops() && Enable256BitVector) {
     addRegisterClass(MVT::v256i1, &PPC::VSRpRCRegClass);
     setOperationAction(ISD::LOAD, MVT::v256i1, Custom);
     setOperationAction(ISD::STORE, MVT::v256i1, Custom);
   }
   if (Subtarget.hasMMA() && Enable512BitVector) {

 bool PPCTargetLowering::preferIncOfAddToSubOfNot(EVT VT) const {
   return VT.isScalarInteger();
 }
 
 const char *PPCTargetLowering::getTargetNodeName(unsigned Opcode) const {
   switch ((PPCISD::NodeType)Opcode) {
   case PPCISD::FIRST_NUMBER:    break;
   case PPCISD::FSEL:            return "PPCISD::FSEL";
-  case PPCISD::XSMAXCDP:        return "PPCISD::XSMAXCDP";
-  case PPCISD::XSMINCDP:        return "PPCISD::XSMINCDP";
+  case PPCISD::XSMAXC:          return "PPCISD::XSMAXC";
+  case PPCISD::XSMINC:          return "PPCISD::XSMINC";
   case PPCISD::FCFID:           return "PPCISD::FCFID";
   case PPCISD::FCFIDU:          return "PPCISD::FCFIDU";
   case PPCISD::FCFIDS:          return "PPCISD::FCFIDS";
   case PPCISD::FCFIDUS:         return "PPCISD::FCFIDUS";
   case PPCISD::FCTIDZ:          return "PPCISD::FCTIDZ";
   case PPCISD::FCTIWZ:          return "PPCISD::FCTIWZ";
   case PPCISD::FCTIDUZ:         return "PPCISD::FCTIDUZ";
   case PPCISD::FCTIWUZ:         return "PPCISD::FCTIWUZ";
SDValue PPCTargetLowering::LowerSELECT_CC(SDValue Op, SelectionDAG &DAG) const {
 
   // Not FP, or using SPE? Not a fsel.
   if (!CmpVT.isFloatingPoint() || !TV.getValueType().isFloatingPoint() ||
       Subtarget.hasSPE())
     return Op;
 
   SDNodeFlags Flags = Op.getNode()->getFlags();
 
-  // We have xsmaxcdp/xsmincdp which are OK to emit even in the
+  // We have xsmaxc[dq]p/xsminc[dq]p which are OK to emit even in the
   // presence of infinities.
   if (Subtarget.hasP9Vector() && LHS == TV && RHS == FV) {
     switch (CC) {
     default:
       break;
     case ISD::SETOGT:
     case ISD::SETGT:
-      return DAG.getNode(PPCISD::XSMAXCDP, dl, Op.getValueType(), LHS, RHS);
+      return DAG.getNode(PPCISD::XSMAXC, dl, Op.getValueType(), LHS, RHS);
     case ISD::SETOLT:
     case ISD::SETLT:
-      return DAG.getNode(PPCISD::XSMINCDP, dl, Op.getValueType(), LHS, RHS);
+      return DAG.getNode(PPCISD::XSMINC, dl, Op.getValueType(), LHS, RHS);
     }
   }
 
   // We might be able to do better than this under some circumstances, but in
   // general, fsel-based lowering of select is a finite-math-only optimization.
   // For more information, see section F.3 of the 2.06 ISA specification.
   // With ISA 3.0
   if ((!DAG.getTarget().Options.NoInfsFPMath && !Flags.hasNoInfs()) ||
       (!DAG.getTarget().Options.NoNaNsFPMath && !Flags.hasNoNaNs()))
     return Op;
 

llvm/lib/Target/PowerPC/PPCInstrInfo.td

 def PPCany_faddrtz: PatFrags<(ops node:$lhs, node:$rhs),
                              [(PPCfaddrtz node:$lhs, node:$rhs),
                               (PPCstrict_faddrtz node:$lhs, node:$rhs)]>;
 
 def PPCfsel   : SDNode<"PPCISD::FSEL",
    // Type constraint for fsel.
    SDTypeProfile<1, 3, [SDTCisSameAs<0, 2>, SDTCisSameAs<0, 3>,
                         SDTCisFP<0>, SDTCisVT<1, f64>]>, []>;
-def PPCxsmaxc : SDNode<"PPCISD::XSMAXCDP", SDT_PPCFPMinMax, []>;
-def PPCxsminc : SDNode<"PPCISD::XSMINCDP", SDT_PPCFPMinMax, []>;
+def PPCxsmaxc : SDNode<"PPCISD::XSMAXC", SDT_PPCFPMinMax, []>;
+def PPCxsminc : SDNode<"PPCISD::XSMINC", SDT_PPCFPMinMax, []>;
 def PPChi       : SDNode<"PPCISD::Hi", SDTIntBinOp, []>;
 def PPClo       : SDNode<"PPCISD::Lo", SDTIntBinOp, []>;
 def PPCtoc_entry: SDNode<"PPCISD::TOC_ENTRY", SDTIntBinOp,
                          [SDNPMayLoad, SDNPMemOperand]>;
 
 def PPCppc32GOT : SDNode<"PPCISD::PPC32_GOT", SDTIntLeaf, []>;
 
 def PPCaddisGotTprelHA : SDNode<"PPCISD::ADDIS_GOT_TPREL_HA", SDTIntBinOp>;
 def PPCldGotTprelL : SDNode<"PPCISD::LD_GOT_TPREL_L", SDTIntBinOp,

llvm/lib/Target/PowerPC/PPCInstrPrefix.td

 
 def VectorInsertions {
   dag LESub0 = (INSERT_SUBREG (v256i1 (IMPLICIT_DEF)), (MTVSRDD $B, $A),
                               sub_vsx0);
   dag LESub1 = (INSERT_SUBREG (v256i1 LESub0), (MTVSRDD $D, $C), sub_vsx1);
 }
 
 let Predicates = [Power10Instrs] in {
-  def XSMAXCQP : X_VT5_VA5_VB5<63, 676, "xsmaxcqp", []>;
-  def XSMINCQP : X_VT5_VA5_VB5<63, 740, "xsmincqp", []>;
+  def XSMAXCQP : X_VT5_VA5_VB5<63, 676, "xsmaxcqp",
+                               [(set f128:$vT, (PPCxsmaxc f128:$vA, f128:$vB))]>;
+  def XSMINCQP : X_VT5_VA5_VB5<63, 740, "xsmincqp",
+                               [(set f128:$vT, (PPCxsminc f128:$vA, f128:$vB))]>;
   def XSCMPEQQP : X_VT5_VA5_VB5<63, 68, "xscmpeqqp", []>;
   def XSCMPGEQP : X_VT5_VA5_VB5<63, 196, "xscmpgeqp", []>;
   def XSCMPGTQP : X_VT5_VA5_VB5<63, 228, "xscmpgtqp", []>;
 }
 
 let Predicates = [Power10Instrs] in {
   def BRH : XForm_11<31, 219, (outs gprc:$RA), (ins gprc:$RS),
                      "brh $RA, $RS", IIC_IntRotate, []>;

llvm/test/CodeGen/PowerPC/scalar-min-max-p10.ll

+; RUN: llc -mcpu=pwr10 -ppc-asm-full-reg-names --enable-unsafe-fp-math \
+; RUN:   -verify-machineinstrs --enable-no-signed-zeros-fp-math \
+; RUN:   --enable-no-nans-fp-math \
+; RUN:   -mtriple=powerpc64le-unknown-unknown < %s | FileCheck %s
+; RUN: llc -mcpu=pwr10 -ppc-asm-full-reg-names -verify-machineinstrs \
+; RUN:   -mtriple=powerpc64le-unknown-unknown < %s | FileCheck %s
+define dso_local fp128 @testqmax(fp128 %a, fp128 %b) local_unnamed_addr {
+; CHECK-LABEL: testqmax:
+; CHECK:       # %bb.0: # %entry
+; CHECK-NEXT:    xsmaxcqp v2, v2, v3
+; CHECK-NEXT:    blr
+;
+entry:
+  %cmp = fcmp ogt fp128 %a, %b
+  %cond = select i1 %cmp, fp128 %a, fp128 %b
+  ret fp128 %cond
+}
+
+define dso_local fp128 @testqmin(fp128 %a, fp128 %b) local_unnamed_addr {
+; CHECK-LABEL: testqmin:
+; CHECK:       # %bb.0: # %entry
+; CHECK-NEXT:    xsmincqp v2, v2, v3
+; CHECK-NEXT:    blr
+;
+entry:
+  %cmp = fcmp olt fp128 %a, %b
+  %cond = select i1 %cmp, fp128 %a, fp128 %b
+  ret fp128 %cond
+}
+
+define dso_local fp128 @testqmax_fast(fp128 %a, fp128 %b) local_unnamed_addr {
+; CHECK-LABEL: testqmax_fast:
+; CHECK:       # %bb.0: # %entry
+; CHECK-NEXT:    xsmaxcqp v2, v2, v3
+; CHECK-NEXT:    blr
+;
+entry:
+  %cmp = fcmp nnan ninf ogt fp128 %a, %b
+  %cond = select i1 %cmp, fp128 %a, fp128 %b
+  ret fp128 %cond
+}
+
+define dso_local fp128 @testqmin_fast(fp128 %a, fp128 %b) local_unnamed_addr {
+; CHECK-LABEL: testqmin_fast:
+; CHECK:       # %bb.0: # %entry
+; CHECK-NEXT:    xsmincqp v2, v2, v3
+; CHECK-NEXT:    blr
+;
+entry:
+  %cmp = fcmp nnan ninf olt fp128 %a, %b
+  %cond = select i1 %cmp, fp128 %a, fp128 %b
+  ret fp128 %cond
+}