[PowerPC] Emit scalar min/max instructions with unsafe fp math

[nemanjai]

Summary

This is something I meant to do a long time ago but never got around to it. These instructions should be an improvement over the compare/fsel sequence we currently emit.

The semantics of the instructions as specified in the ISA match the semantics specified in the description of the nodes.

[jsji]

It is a great idea to exploit xsmindp/xsmaxdp! But looks like we make it more general than restricted to UnsafeFPMath?

llvm/lib/Target/PowerPC/PPCISelLowering.cpp

550 ↗

(On Diff #203487)

Why we need TM.Options.UnsafeFPMath here?
If ISD::FMAXNUM_IEEE is generated, then the semantic is exact the same as `xsmaxdp,
we should be safe to use xsmaxdp.

I think we can also add actions for ISD::FMAXNUM/FMAXIMUM and ISD::FMINNUM/FMINIMUM,
then we do need TM.Options.UnsafeFPMath or TM.Options.NoNansFPMath/NoSignedZerosFPMath for them.

llvm/lib/Target/PowerPC/PPCInstrVSX.td

984 ↗

(On Diff #203487)

We can add similar patterns for fmaxnum/fmaxinum and fminnum/fmininum with Predicates ?

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

2 ↗

(On Diff #203487)

Maybe add RUN lines for --enable-no-nans-fp-math/-enable-no-signed-zeros-fp-math?

7 ↗

(On Diff #203487)

It would be great if we can pre-commit the testcase to show only difference .

48 ↗

(On Diff #203487)

Why we need these attributes? Looks like these should be in different RUN line ?

[jsji]

Move out of review queue, need author's action.

[amyk]

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

2 ↗

(On Diff #203487)

Maybe also add -verify-machineinstrs to our tests?

[nemanjai]

llvm/lib/Target/PowerPC/PPCISelLowering.cpp

550 ↗

(On Diff #203487)

When I originally did this, it would produce these nodes along with ISD::FCANONICALIZE when unsafe fp math isn't specified. However, the DAG combiner seems to have been modified to not do that any longer. The instructions themselves handle SNaNs correctly anyway so we can handle the inputs coming from ISD::FCANONICALIZE anyway.

However, I don't really see a point in legalizing FMAXNUM/FMINNUM since we will just get the _IEEE versions even with fast math.

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

7 ↗

(On Diff #203487)

I will add a RUN line without the FMF flags which will show the difference in codegen in the test case itself.

[nemanjai]

Remove the requirement for unsafe math for the FMAXNUM_IEEE and FMINNUM_IEEE. Add codegen for P9 xsmaxdp/xsmindp. Improve the test case.

[steven.zhang]

I see there is one sanity failed.

FAIL: LLVM :: CodeGen/PowerPC/ctr-minmaxnum.ll (29517 of 50289)
******************** TEST 'LLVM :: CodeGen/PowerPC/ctr-minmaxnum.ll' FAILED ********************
Script:
--
: 'RUN: at line 1';   /home/qshanz/work/build/bin/llc -mtriple=powerpc64-unknown-linux-gnu -verify-machineinstrs -mcpu=pwr7 < /home/qshanz/work/llvm/llvm/test/CodeGen/PowerPC/ctr-minmaxnum.ll | /home/qshanz/work/build/bin/FileCheck /home/qshanz/work/llvm/llvm/test/CodeGen/PowerPC/ctr-minmaxnum.ll
: 'RUN: at line 2';   /home/qshanz/work/build/bin/llc -mtriple=powerpc64-unknown-linux-gnu -verify-machineinstrs -mcpu=a2q < /home/qshanz/work/llvm/llvm/test/CodeGen/PowerPC/ctr-minmaxnum.ll | /home/qshanz/work/build/bin/FileCheck /home/qshanz/work/llvm/llvm/test/CodeGen/PowerPC/ctr-minmaxnum.ll --check-prefix=QPX
--
Exit Code: 2

Command Output (stderr):
--
LLVM ERROR: Cannot select: t14: f32 = fcanonicalize t2
  t2: f32,ch = CopyFromReg t0, Register:f32 %2
    t1: f32 = Register %2
In function: test1
FileCheck error: '-' is empty.
FileCheck command line:  /home/qshanz/work/build/bin/FileCheck /home/qshanz/work/llvm/llvm/test/CodeGen/PowerPC/ctr-minmaxnum.ll

lib/Target/PowerPC/PPCISelLowering.cpp

555

We will get the ISD::FMAXNUM/ISD::FMINNUM node if mark it as legal.

define dso_local float @testfmax_fast(float %a, float %b) {
entry:
  %cmp = fcmp fast ogt float %a, %b
  %cond = select i1 %cmp, float %a, float %b
  ret float %cond
}

llc test.ll -mattr=+vsx

And also for the intrinsic llvm.minnum/llvm.maxnum.

Initial selection DAG: %bb.0 'testfmax_fast:entry'
SelectionDAG has 11 nodes:
  t0: ch = EntryToken
  t2: f32,ch = CopyFromReg t0, Register:f32 %0
  t4: f32,ch = CopyFromReg t0, Register:f32 %1
  t6: i1 = setcc nnan ninf nsz arcp contract afn reassoc t2, t4, setgt:ch
    t7: f32 = fmaxnum t2, t4
  t9: ch,glue = CopyToReg t0, Register:f32 $f1, t7
  t10: ch = PPCISD::RET_FLAG t9, Register:f32 $f1, t9:1

The node is built directly not combined by select_cc. And I think, we need to lower it if we know that, the operand is NaN or not(i.e. isKnownNeverNaN()).

7221

Is it hasVSX() more clear ?

7225

Do we need logic to handle the case that if the op is NaN ?

If src1 or src2 is a SNaN, an Invalid Operation
exception occurs.
If either src1 or src2 is a NaN, result is src2.
Otherwise, if src1 is less than src2, result is src1.
Otherwise, result is src2.

The ISA documentation is a bit confusing here. Isn't NaN including SNaN and QNaN ? The condition in the second if cover the first one.

lib/Target/PowerPC/PPCInstrInfo.td

120

SDT_PPCFPMinMax ?

[steven.zhang]

lib/Target/PowerPC/PPCISelLowering.cpp

555

Hmm, ignore the above comments. It is right to have DAG generated the IEEE node instead of the non-IEEE, as the hw has instruction semantics equal.

7225

Please also ignore above comments as after double confirm, the XSMAXCDP/XSMINCDP perfectly match the semantics, no matter if the operand is NaN or not.

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

12

It would be great if we have some test to verify the behavior if operand is SNaN/QNaN for P9. However, this is NOT a must.

[nemanjai]

In D62993#1718495, @steven.zhang wrote:

I see there is one sanity failed.
LLVM ERROR: Cannot select: t14: f32 = fcanonicalize t2

t2: f32,ch = CopyFromReg t0, Register:f32 %2
  t1: f32 = Register %2

In function: test1
FileCheck error: '-' is empty.
FileCheck command line: /home/qshanz/work/build/bin/FileCheck /home/qshanz/work/llvm/llvm/test/CodeGen/PowerPC/ctr-minmaxnum.ll

Ah, that's where I saw the issue I mentioned in https://reviews.llvm.org/D62993?id=203487#inline-622927
I'll fix it up in the next update.

lib/Target/PowerPC/PPCISelLowering.cpp

555

I don't dispute that we will get the nodes if we mark them legal. However, I do not think that we will get these nodes in more situations than we get the _IEEE versions.

The way I see it, with ninf nsz nnan, the nodes are equivalent since the only difference between them is the handling of NaNs and (presumably) signed zeros.

7205

This needs to change from isISA3_0() to hasP9Vector().

7221

I dont' understand this comment. This is only available in ISA3.0, so hasVSX() is not adequate. And VSX is a requirement for P9Vector, so why would I need both?

7225

Quiet NaNs are fine. Signaling NaNs cause exceptions. This is fine, signaling NaNs are supposed to cause exceptions as far as I can tell.

lib/Target/PowerPC/PPCInstrInfo.td

120

Sounds good. Will do.

[nemanjai]

Add handling for ISD::FCANONICALIZE.
Add a few tests.
Fix up some naming.

[steven.zhang]

LGTM from my aspect.

[steven.zhang]

lib/Target/PowerPC/PPCInstrVSX.td

1280

Just out of curious, why we set the complexity as 1 here instead of 400.

[nemanjai]

lib/Target/PowerPC/PPCInstrVSX.td

1280

Ha ha... because it's a bug. I'll change it to 400 on the commit.
It doesn't change semantics because there isn't an equivalent FPU choice to be made here, but it should still be consistent.

[nemanjai]

@jsji Any further comments?

[jsji]

Some nit, the biggest question is with line 7223

lib/Target/PowerPC/PPCISelLowering.cpp

1304

Typo? XSMAXCDP not XSMAXCPD.

7210

Although this won't have problem right now, because we always return before using Flags.
I think it would be better to move this after the new code, to avoid a potential trap for future programmers.

7223

We will lose some opportunities here, eg: with -mcpu=pwr9 --enable-no-nans-fp-math --enable-no-signed-zeros-fp-math?
We will catch new opportunities for max/min, but will give up lowering all the other CC?

lib/Target/PowerPC/PPCInstrVSX.td

1303

Nit: The pattern order here is not consistent with above: instead of having one min, one max, we start to put all min together, hen max. It won't have problem, but would be better to read/check if we make it consistent.

test/CodeGen/PowerPC/ctr-minmaxnum.ll

69 ↗

(On Diff #226125)

QPX should NOT be affected, so shouldn't change here?

144 ↗

(On Diff #226125)

QPX , shouldn't change.

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

3

I would be more interested to see -mcpu=pwr9 with --enable-no-nans-fp-math instead of -mcpu=pwr8. :)

[nemanjai]

lib/Target/PowerPC/PPCISelLowering.cpp

1304

Oops. Thanks, I'l fix it.

7210

Fair enough, I'll move this down below the min/max switch.

7223

That is a really good point, this will prevent us from generating the fsel on P9. I'll fix it up.

lib/Target/PowerPC/PPCInstrVSX.td

1303

I agree 100% that it's nicer to keep it organized and consistent. Will update, thank you.

test/CodeGen/PowerPC/ctr-minmaxnum.ll

69 ↗

(On Diff #226125)

Oops, overzealous search-and-replace...

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

3

I think it is good to show that we get the same instructions with fast math on both P8 and P9.

[nemanjai]

Address the minor comments and fix the early exit on P9 with no NaNs/Infs.

[jsji]

LGTM. Thanks.

[jsji]

Oh, maybe the title should be updated to remove "unsafe fp math" to avoid confusion?

lib/Target/PowerPC/PPCISelLowering.h

namespace PPCISD {
     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,
+
       /// 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.
       FCFIDU, FCFIDS, FCFIDUS,

lib/Target/PowerPC/PPCISelLowering.cpp

if (Subtarget.use64BitRegs()) {
     setOperationAction(ISD::SRL_PARTS, MVT::i64, Custom);
   } else {
     // 32-bit PowerPC wants to expand i64 shifts itself.
     setOperationAction(ISD::SHL_PARTS, MVT::i32, Custom);
     setOperationAction(ISD::SRA_PARTS, MVT::i32, Custom);
     setOperationAction(ISD::SRL_PARTS, MVT::i32, Custom);
   }
 
+  if (Subtarget.hasVSX()) {
+    setOperationAction(ISD::FMAXNUM_IEEE, MVT::f64, Legal);
+    setOperationAction(ISD::FMAXNUM_IEEE, MVT::f32, Legal);
+    setOperationAction(ISD::FMINNUM_IEEE, MVT::f64, Legal);
+    setOperationAction(ISD::FMINNUM_IEEE, MVT::f32, Legal);
+  }
+
   if (Subtarget.hasAltivec()) {
     // First set operation action for all vector types to expand. Then we
     // will selectively turn on ones that can be effectively codegen'd.
     for (MVT VT : MVT::fixedlen_vector_valuetypes()) {
       // add/sub are legal for all supported vector VT's.
       setOperationAction(ISD::ADD, VT, Legal);
       setOperationAction(ISD::SUB, VT, Legal);
 

 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::XSMAXCPD";
+  case PPCISD::XSMINCDP:        return "PPCISD::XSMINCDP";
   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? Not a fsel.
   if (!Op.getOperand(0).getValueType().isFloatingPoint() ||
       !Op.getOperand(2).getValueType().isFloatingPoint())
     return Op;
 
   // 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.
-  if (!DAG.getTarget().Options.NoInfsFPMath ||
-      !DAG.getTarget().Options.NoNaNsFPMath)
+  // With ISA 3.0, we have xsmaxcdp/xsmincdp which are OK to emit even in the
+  // presence of infinities.
+  if (!Subtarget.isISA3_0() && (!DAG.getTarget().Options.NoInfsFPMath ||
+                                !DAG.getTarget().Options.NoNaNsFPMath))
     return Op;
   // TODO: Propagate flags from the select rather than global settings.
   SDNodeFlags Flags;
   Flags.setNoInfs(true);
   Flags.setNoNaNs(true);
 
   ISD::CondCode CC = cast<CondCodeSDNode>(Op.getOperand(4))->get();
 
   EVT ResVT = Op.getValueType();
   EVT CmpVT = Op.getOperand(0).getValueType();
   SDValue LHS = Op.getOperand(0), RHS = Op.getOperand(1);
   SDValue TV  = Op.getOperand(2), FV  = Op.getOperand(3);
   SDLoc dl(Op);
 
+  if (Subtarget.hasP9Vector() && LHS == TV && RHS == FV) {
+    switch (CC) {
+    default: return Op;
+    case ISD::SETOGT:
+      return DAG.getNode(PPCISD::XSMAXCDP, dl, Op.getValueType(), LHS, RHS);
+    case ISD::SETOLT:
+      return DAG.getNode(PPCISD::XSMINCDP, dl, Op.getValueType(), LHS, RHS);
+    }
+  }
+
   // If the RHS of the comparison is a 0.0, we don't need to do the
   // subtraction at all.
   SDValue Sel1;
   if (isFloatingPointZero(RHS))
     switch (CC) {
     default: break;       // SETUO etc aren't handled by fsel.
     case ISD::SETNE:
       std::swap(TV, FV);

lib/Target/PowerPC/PPCInstrInfo.td

 def SDT_PPCqvlfsb : SDTypeProfile<1, 1, [
   SDTCisVec<0>, SDTCisPtrTy<1>
 ]>;
 
 def SDT_PPCextswsli : SDTypeProfile<1, 2, [  // extswsli
   SDTCisInt<0>, SDTCisInt<1>, SDTCisOpSmallerThanOp<1, 0>, SDTCisInt<2>
 ]>;
 
+def SDT_FPMinMax : SDTypeProfile<1, 2, [
+  SDTCisSameAs<0, 1>, SDTCisSameAs<0, 2>, SDTCisFP<0>
+]>;
+
 //===----------------------------------------------------------------------===//
 // PowerPC specific DAG Nodes.
 //
 
 def PPCfre    : SDNode<"PPCISD::FRE",     SDTFPUnaryOp, []>;
 def PPCfrsqrte: SDNode<"PPCISD::FRSQRTE", SDTFPUnaryOp, []>;
 
 def PPCfcfid  : SDNode<"PPCISD::FCFID",   SDTFPUnaryOp, []>;

 // Perform FADD in round-to-zero mode.
 def PPCfaddrtz: SDNode<"PPCISD::FADDRTZ", SDTFPBinOp, []>;
 
 
 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_FPMinMax, []>;
+def PPCxsminc : SDNode<"PPCISD::XSMINCDP", SDT_FPMinMax, []>;
 def PPChi       : SDNode<"PPCISD::Hi", SDTIntBinOp, []>;
 def PPClo       : SDNode<"PPCISD::Lo", SDTIntBinOp, []>;
 def PPCtoc_entry: SDNode<"PPCISD::TOC_ENTRY", SDTIntBinOp,
                          [SDNPMayLoad, SDNPMemOperand]>;
 def PPCvmaddfp  : SDNode<"PPCISD::VMADDFP", SDTFPTernaryOp, []>;
 def PPCvnmsubfp : SDNode<"PPCISD::VNMSUBFP", SDTFPTernaryOp, []>;
 
 def PPCppc32GOT : SDNode<"PPCISD::PPC32_GOT", SDTIntLeaf, []>;

lib/Target/PowerPC/PPCInstrVSX.td

 let AddedComplexity = 400 in { // Prefer VSX patterns over non-VSX patterns.
 
 def : Pat<(v4i32 (vnot_ppc v4i32:$A)),
           (v4i32 (XXLNOR $A, $A))>;
 def : Pat<(v4i32 (or (and (vnot_ppc v4i32:$C), v4i32:$A),
                      (and v4i32:$B, v4i32:$C))),
           (v4i32 (XXSEL $A, $B, $C))>;
 
+def : Pat<(f32 (fminnum_ieee f32:$A, f32:$B)),
+          (f32 (COPY_TO_REGCLASS (XSMINDP (COPY_TO_REGCLASS $A, VSFRC),
+                                          (COPY_TO_REGCLASS $B, VSFRC)),
+                                 VSSRC))>;
+def : Pat<(f32 (fmaxnum_ieee f32:$A, f32:$B)),
+          (f32 (COPY_TO_REGCLASS (XSMAXDP (COPY_TO_REGCLASS $A, VSFRC),
+                                          (COPY_TO_REGCLASS $B, VSFRC)),
+                                 VSSRC))>;
+def : Pat<(f64 (fminnum_ieee f64:$A, f64:$B)),
+          (f64 (XSMINDP $A, $B))>;
+def : Pat<(f64 (fmaxnum_ieee f64:$A, f64:$B)),
+          (f64 (XSMAXDP $A, $B))>;
+
 let Predicates = [IsBigEndian] in {
 def : Pat<(v2f64 (scalar_to_vector f64:$A)),
           (v2f64 (SUBREG_TO_REG (i64 1), $A, sub_64))>;
 
 def : Pat<(f64 (extractelt v2f64:$S, 0)),
           (f64 (EXTRACT_SUBREG $S, sub_64))>;
 def : Pat<(f64 (extractelt v2f64:$S, 1)),
           (f64 (EXTRACT_SUBREG (XXPERMDI $S, $S, 2), sub_64))>;
def ScalarLoads {
   dag Li8 =       (i32 (extloadi8 xoaddr:$src));
   dag ZELi8 =     (i32 (zextloadi8 xoaddr:$src));
   dag ZELi8i64 =  (i64 (zextloadi8 xoaddr:$src));
   dag SELi8 =     (i32 (sext_inreg (extloadi8 xoaddr:$src), i8));
   dag SELi8i64 =  (i64 (sext_inreg (extloadi8 xoaddr:$src), i8));
 
   dag Li16 =      (i32 (extloadi16 xoaddr:$src));
   dag ZELi16 =    (i32 (zextloadi16 xoaddr:$src));
   dag ZELi16i64 = (i64 (zextloadi16 xoaddr:$src));
   dag SELi16 =    (i32 (sextloadi16 xoaddr:$src));
   dag SELi16i64 = (i64 (sextloadi16 xoaddr:$src));
 
   dag Li32 = (i32 (load xoaddr:$src));
 }
 
 def DWToSPExtractConv {
   dag El0US1 = (f32 (PPCfcfidus
                     (f64 (PPCmtvsra (i64 (vector_extract v2i64:$S1, 0))))));
   dag El1US1 = (f32 (PPCfcfidus
                     (f64 (PPCmtvsra (i64 (vector_extract v2i64:$S1, 1))))));
   dag El0US2 = (f32 (PPCfcfidus
                     (f64 (PPCmtvsra (i64 (vector_extract v2i64:$S2, 0))))));
   dag El1US2 = (f32 (PPCfcfidus
                     (f64 (PPCmtvsra (i64 (vector_extract v2i64:$S2, 1))))));
   dag El0SS1 = (f32 (PPCfcfids
                     (f64 (PPCmtvsra (i64 (vector_extract v2i64:$S1, 0))))));
   dag El1SS1 = (f32 (PPCfcfids
                     (f64 (PPCmtvsra (i64 (vector_extract v2i64:$S1, 1))))));
   dag El0SS2 = (f32 (PPCfcfids
                     (f64 (PPCmtvsra (i64 (vector_extract v2i64:$S2, 0))))));
   dag El1SS2 = (f32 (PPCfcfids
                     (f64 (PPCmtvsra (i64 (vector_extract v2i64:$S2, 1))))));
   dag BVU = (v4f32 (build_vector El0US1, El1US1, El0US2, El1US2));
   dag BVS = (v4f32 (build_vector El0SS1, El1SS1, El0SS2, El1SS2));
 }
 
 // The following VSX instructions were introduced in Power ISA 2.07
 /* FIXME: if the operands are v2i64, these patterns will not match.
    we should define new patterns or otherwise match the same patterns
    when the elements are larger than i32.
def XVTSTDCDP : XX2_RD6_DCMX7_RS6<60, 15, 5,
                               (outs vsrc:$XT), (ins u7imm:$DCMX, vsrc:$XB),
                               "xvtstdcdp $XT, $XB, $DCMX", IIC_VecFP,
                               [(set v2i64: $XT,
                                (int_ppc_vsx_xvtstdcdp v2f64:$XB, timm:$DCMX))]>;
 
   //===--------------------------------------------------------------------===//
 
   // Maximum/Minimum Type-C/Type-J DP
-  // XT.dword[1] = 0xUUUU_UUUU_UUUU_UUUU, so we use vsrc for XT
-  def XSMAXCDP : XX3_XT5_XA5_XB5<60, 128, "xsmaxcdp", vsrc, vsfrc, vsfrc,
-                                 IIC_VecFP, []>;
+  def XSMAXCDP : XX3_XT5_XA5_XB5<60, 128, "xsmaxcdp", vsfrc, vsfrc, vsfrc,
+                                 IIC_VecFP,
+                                 [(set f64:$XT, (PPCxsmaxc f64:$XA, f64:$XB))]>;
   def XSMAXJDP : XX3_XT5_XA5_XB5<60, 144, "xsmaxjdp", vsrc, vsfrc, vsfrc,
                                  IIC_VecFP, []>;
-  def XSMINCDP : XX3_XT5_XA5_XB5<60, 136, "xsmincdp", vsrc, vsfrc, vsfrc,
-                                 IIC_VecFP, []>;
+  def XSMINCDP : XX3_XT5_XA5_XB5<60, 136, "xsmincdp", vsfrc, vsfrc, vsfrc,
+                                 IIC_VecFP,
+                                 [(set f64:$XT, (PPCxsminc f64:$XA, f64:$XB))]>;
   def XSMINJDP : XX3_XT5_XA5_XB5<60, 152, "xsminjdp", vsrc, vsfrc, vsfrc,
                                  IIC_VecFP, []>;
 
   //===--------------------------------------------------------------------===//
 
   // Vector Byte-Reverse H/W/D/Q Word
   def XXBRH : XX2_XT6_XO5_XB6<60,  7, 475, "xxbrh", vsrc, []>;
   def XXBRW : XX2_XT6_XO5_XB6<60, 15, 475, "xxbrw", vsrc, []>;

   // Round & Convert QP -> DP/SP
   def : Pat<(f64 (fpround f128:$src)), (f64 (XSCVQPDP $src))>;
   def : Pat<(f32 (fpround f128:$src)), (f32 (XSRSP (XSCVQPDPO $src)))>;
 
   // Convert SP -> QP
   def : Pat<(f128 (fpextend f32:$src)),
             (f128 (XSCVDPQP (COPY_TO_REGCLASS $src, VFRC)))>;
 
+  def : Pat<(f32 (PPCxsmaxc f32:$XA, f32:$XB)),
+            (f32 (COPY_TO_REGCLASS (XSMAXCDP (COPY_TO_REGCLASS $XA, VSSRC),
+                                             (COPY_TO_REGCLASS $XB, VSSRC)),
+                                   VSSRC))>;
+  def : Pat<(f32 (PPCxsminc f32:$XA, f32:$XB)),
+            (f32 (COPY_TO_REGCLASS (XSMINCDP (COPY_TO_REGCLASS $XA, VSSRC),
+                                             (COPY_TO_REGCLASS $XB, VSSRC)),
+                                   VSSRC))>;
+
 } // end HasP9Vector, AddedComplexity
 
 let AddedComplexity = 400 in {
   let Predicates = [IsISA3_0, HasP9Vector, HasDirectMove, IsBigEndian] in {
     def : Pat<(f128 (PPCbuild_fp128 i64:$rB, i64:$rA)),
               (f128 (COPY_TO_REGCLASS (MTVSRDD $rB, $rA), VRRC))>;
   }
   let Predicates = [IsISA3_0, HasP9Vector, HasDirectMove, IsLittleEndian] in {

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

+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc -mcpu=pwr8 -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=pwr9 -ppc-asm-full-reg-names -verify-machineinstrs \
+; RUN:   -mtriple=powerpc64le-unknown-unknown < %s | FileCheck %s \
+; RUN:   --check-prefix=NO-FAST-P9
+; RUN: llc -mcpu=pwr8 -ppc-asm-full-reg-names -verify-machineinstrs \
+; RUN:   -mtriple=powerpc64le-unknown-unknown < %s | FileCheck %s \
+; RUN:   --check-prefix=NO-FAST-P8
+define dso_local float @testfmax(float %a, float %b) local_unnamed_addr #0 {
+; CHECK-LABEL: testfmax:
+; CHECK:       # %bb.0: # %entry
+; CHECK-NEXT:    xsmaxdp f1, f1, f2
+; CHECK-NEXT:    blr
+;
+; NO-FAST-P9-LABEL: testfmax:
+; NO-FAST-P9:       # %bb.0: # %entry
+; NO-FAST-P9-NEXT:    xsmaxcdp f1, f1, f2
+; NO-FAST-P9-NEXT:    blr
+;
+; NO-FAST-P8-LABEL: testfmax:
+; NO-FAST-P8:       # %bb.0: # %entry
+; NO-FAST-P8-NEXT:    fcmpu cr0, f1, f2
+; NO-FAST-P8-NEXT:    bgtlr cr0
+; NO-FAST-P8-NEXT:  # %bb.1: # %entry
+; NO-FAST-P8-NEXT:    fmr f1, f2
+; NO-FAST-P8-NEXT:    blr
+entry:
+  %cmp = fcmp ogt float %a, %b
+  %cond = select i1 %cmp, float %a, float %b
+  ret float %cond
+}
+
+define dso_local double @testdmax(double %a, double %b) local_unnamed_addr #0 {
+; CHECK-LABEL: testdmax:
+; CHECK:       # %bb.0: # %entry
+; CHECK-NEXT:    xsmaxdp f1, f1, f2
+; CHECK-NEXT:    blr
+;
+; NO-FAST-P9-LABEL: testdmax:
+; NO-FAST-P9:       # %bb.0: # %entry
+; NO-FAST-P9-NEXT:    xsmaxcdp f1, f1, f2
+; NO-FAST-P9-NEXT:    blr
+;
+; NO-FAST-P8-LABEL: testdmax:
+; NO-FAST-P8:       # %bb.0: # %entry
+; NO-FAST-P8-NEXT:    xscmpudp cr0, f1, f2
+; NO-FAST-P8-NEXT:    bgtlr cr0
+; NO-FAST-P8-NEXT:  # %bb.1: # %entry
+; NO-FAST-P8-NEXT:    fmr f1, f2
+; NO-FAST-P8-NEXT:    blr
+entry:
+  %cmp = fcmp ogt double %a, %b
+  %cond = select i1 %cmp, double %a, double %b
+  ret double %cond
+}
+
+define dso_local float @testfmin(float %a, float %b) local_unnamed_addr #0 {
+; CHECK-LABEL: testfmin:
+; CHECK:       # %bb.0: # %entry
+; CHECK-NEXT:    xsmindp f1, f1, f2
+; CHECK-NEXT:    blr
+;
+; NO-FAST-P9-LABEL: testfmin:
+; NO-FAST-P9:       # %bb.0: # %entry
+; NO-FAST-P9-NEXT:    xsmincdp f1, f1, f2
+; NO-FAST-P9-NEXT:    blr
+;
+; NO-FAST-P8-LABEL: testfmin:
+; NO-FAST-P8:       # %bb.0: # %entry
+; NO-FAST-P8-NEXT:    fcmpu cr0, f1, f2
+; NO-FAST-P8-NEXT:    bltlr cr0
+; NO-FAST-P8-NEXT:  # %bb.1: # %entry
+; NO-FAST-P8-NEXT:    fmr f1, f2
+; NO-FAST-P8-NEXT:    blr
+entry:
+  %cmp = fcmp olt float %a, %b
+  %cond = select i1 %cmp, float %a, float %b
+  ret float %cond
+}
+
+define dso_local double @testdmin(double %a, double %b) local_unnamed_addr #0 {
+; CHECK-LABEL: testdmin:
+; CHECK:       # %bb.0: # %entry
+; CHECK-NEXT:    xsmindp f1, f1, f2
+; CHECK-NEXT:    blr
+;
+; NO-FAST-P9-LABEL: testdmin:
+; NO-FAST-P9:       # %bb.0: # %entry
+; NO-FAST-P9-NEXT:    xsmincdp f1, f1, f2
+; NO-FAST-P9-NEXT:    blr
+;
+; NO-FAST-P8-LABEL: testdmin:
+; NO-FAST-P8:       # %bb.0: # %entry
+; NO-FAST-P8-NEXT:    xscmpudp cr0, f1, f2
+; NO-FAST-P8-NEXT:    bltlr cr0
+; NO-FAST-P8-NEXT:  # %bb.1: # %entry
+; NO-FAST-P8-NEXT:    fmr f1, f2
+; NO-FAST-P8-NEXT:    blr
+entry:
+  %cmp = fcmp olt double %a, %b
+  %cond = select i1 %cmp, double %a, double %b
+  ret double %cond
+}