Convert clamp into fmaxnum/fminnum pairs.

[sebpop]

Summary

This patch converts clamp at DAG level in DAGCombiner.cpp. For example, the following code:

float clamp(float a)                                                                                                                                                                       
{                                                                                                                                                                                             
  const float b = 255.0;                                                                                                                                                                      
  const float c = 0.0;                                                                                                                                                                        
                                                                                                                                                                                              
  if(b < a)                                                                                                                                                                                   
    return b;                                                                                                                                                                                 
  if(a < c)                                                                                                                                                                                   
    return c;                                                                                                                                                                                 
                                                                                                                                                                                              
  return a;                                                                                                                                                                                   
}

could be compiled:

clamp:                                  // @clamp
// BB#0:                                // %entry
        adrp    x8, .LCPI0_0
        ldr     s1, [x8, :lo12:.LCPI0_0]
        fmov    s2, wzr
        fmaxnm  s0, s0, s2
        fminnm  s0, s0, s1
        ret

I work with @hiraditya on this patch.

[hxy9243]

Ping. Are there any updates or thoughts on this patch? Thanks.

[arsenm]

llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp

6558–6559

The || should go at the end of the line

6558–6559

Why are you checking the number of operands? You can just check that it's SETCC

6561

Don't need the get node (and actually might be broken)

6565

You don't need the get node

6567

Return on separate line

6572–6575

You don't need this, the DAG is broken if this ever happens

6586–6587

Ditto

6591

unchecked dynast, used cast<>

6875–6876

This should probably only return if combineSelectFP succeeded

llvm/test/CodeGen/AArch64/aarch64-DAGCombine-fminmax.ll

2

Don't need -O=3

2

This needs to be run without unsafe math to make sure it isn't happening without it

3

Comment is wrong

6

This is redundant with the run line and should be removed

54

Missing a testcase where there are the minnum/maxnum intrinsic calls instead of ones formed from unsafe cmp + select

55

Extra string attributes should be removed

57–59

Should be removed

[hxy9243]

Address comments from @arsenm.

[hxy9243]

llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp

6572–6575

These are separate instructions, and could have different semantics. The comparison will be broken if we don't check semantics.

6586–6587

Ditto.

[hxy9243]

Ping. Are there any updates on this? Thanks very much.

[hxy9243]

Ping agian. Thanks in advance for the reviews.

[sebpop]

Updated patch. I will post perf numbers on some benchmarks with this patch.

[sebpop]

In the following experiment a positive number is an increase in performance,
the best score was taken out of 3 runs on firefly aarch64 A-72:

spec2000-164.gzip	0.02%
spec2000-175.vpr	-0.55%
spec2000-177.mesa	-0.18%
spec2000-179.art	-0.38%
spec2000-181.mcf	-0.88%
spec2000-183.equake	-0.06%
spec2000-186.crafty	-0.13%
spec2000-188.ammp	-1.06%
spec2000-197.parser	-0.54%
spec2000-252.eon	-0.49%
spec2000-253.perlbmk	0.17%
spec2000-254.gap	-0.04%
spec2000-255.vortex	0.26%
spec2000-256.bzip2	-0.50%
spec2000-300.twolf	0.38%

[arsenm]

Is this still relevant?

llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp

private:
     void ReplaceLoadWithPromotedLoad(SDNode *Load, SDNode *ExtLoad);
     SDValue PromoteOperand(SDValue Op, EVT PVT, bool &Replace);
     SDValue SExtPromoteOperand(SDValue Op, EVT PVT);
     SDValue ZExtPromoteOperand(SDValue Op, EVT PVT);
     SDValue PromoteIntBinOp(SDValue Op);
     SDValue PromoteIntShiftOp(SDValue Op);
     SDValue PromoteExtend(SDValue Op);
     bool PromoteLoad(SDValue Op);
+    SDValue combineSelectFP(SDNode *N);
 
     /// Call the node-specific routine that knows how to fold each
     /// particular type of node. If that doesn't do anything, try the
     /// target-specific DAG combines.
     SDValue combine(SDNode *N);
 
     // Visitation implementation - Implement dag node combining for different
     // node types.  The semantics are as follows:
SDValue DAGCombiner::foldSelectOfConstants(SDNode *N) {
   SDValue N2 = N->getOperand(2);
   EVT VT = N->getValueType(0);
   EVT CondVT = Cond.getValueType();
   SDLoc DL(N);
 
   if (!VT.isInteger())
     return SDValue();
 
   auto *C1 = dyn_cast<ConstantSDNode>(N1);
   auto *C2 = dyn_cast<ConstantSDNode>(N2);
   if (!C1 || !C2)
     return SDValue();
 
   // Only do this before legalization to avoid conflicting with target-specific
   // transforms in the other direction (create a select from a zext/sext). There
   // is also a target-independent combine here in DAGCombiner in the other
   // direction for (select Cond, -1, 0) when the condition is not i1.
   if (CondVT == MVT::i1 && !LegalOperations) {
     if (C1->isNullValue() && C2->isOne()) {
       // select Cond, 0, 1 --> zext (!Cond)
       SDValue NotCond = DAG.getNOT(DL, Cond, MVT::i1);
       if (VT != MVT::i1)
         NotCond = DAG.getNode(ISD::ZERO_EXTEND, DL, VT, NotCond);
       return NotCond;
     }
     if (C1->isNullValue() && C2->isAllOnesValue()) {
       // select Cond, 0, -1 --> sext (!Cond)
       SDValue NotCond = DAG.getNOT(DL, Cond, MVT::i1);
       if (VT != MVT::i1)
         NotCond = DAG.getNode(ISD::SIGN_EXTEND, DL, VT, NotCond);
       return NotCond;
     }
     if (C1->isOne() && C2->isNullValue()) {
       // select Cond, 1, 0 --> zext (Cond)
       if (VT != MVT::i1)
         Cond = DAG.getNode(ISD::ZERO_EXTEND, DL, VT, Cond);
       return Cond;
     }
     if (C1->isAllOnesValue() && C2->isNullValue()) {
       // select Cond, -1, 0 --> sext (Cond)
       if (VT != MVT::i1)
         Cond = DAG.getNode(ISD::SIGN_EXTEND, DL, VT, Cond);
       return Cond;
     }
 
     // For any constants that differ by 1, we can transform the select into an
     // extend and add. Use a target hook because some targets may prefer to
     // transform in the other direction.
     if (TLI.convertSelectOfConstantsToMath(VT)) {
       if (C1->getAPIntValue() - 1 == C2->getAPIntValue()) {
if (CondVT.isInteger() &&
     if (VT.bitsEq(CondVT))
       return NotCond;
     return DAG.getZExtOrTrunc(NotCond, DL, VT);
   }
 
   return SDValue();
 }
 
+// Perform checks on select instructions, and replace it with
+// fmin or fmax.
+SDValue DAGCombiner::combineSelectFP(SDNode *N) {
+  SDValue N0 = N->getOperand(0);
+  SDValue N1 = N->getOperand(1);
+  SDValue N2 = N->getOperand(2);
+
+  EVT VT = N->getValueType(0);
+  SDNode *CmpNode = N0.getNode();
+
+  // Check if num of operands match requirements
+  if (N2->getNumOperands() < 2 || CmpNode->getOpcode() != ISD::SETCC ||
+      CmpNode->getOperand(0) != N2.getOperand(0))
+    return SDValue();
+
+  // Is it really safe here?
+  ConstantFPSDNode *CmpN1FP =
+      dyn_cast<ConstantFPSDNode>(CmpNode->getOperand(1));
+  ConstantFPSDNode *N1FP = dyn_cast<ConstantFPSDNode>(N1);
+  if (!CmpN1FP || !N1FP)
+    return SDValue();
+
+  const APFloat &CmpN1FPVal = CmpN1FP->getValueAPF();
+  const APFloat &N1FPVal = N1FP->getValueAPF();
+
+  // Check if float point constant are the same
+  if (&CmpN1FPVal.getSemantics() != &N1FPVal.getSemantics() ||
+      CmpN1FPVal.compare(N1FPVal) != APFloat::cmpEqual)
+    return SDValue();
+
+  // Check if the N2 operand is constant float
+  SDNode *N2Node = N2.getNode();
+  ConstantFPSDNode *N2Operand2 =
+      dyn_cast<ConstantFPSDNode>(N2Node->getOperand(1).getNode());
+  if (!N2Operand2)
+    return SDValue();
+
+  // Check the value of cmpN1 and N2 are equal
+  APFloat N2Operand2Val = N2Operand2->getValueAPF();
+  if (&CmpN1FPVal.getSemantics() != &N2Operand2Val.getSemantics())
+    return SDValue();
+
+  unsigned RetOpcode;
+  // Switch based on the comparison operand.
+  switch (cast<CondCodeSDNode>(CmpNode->getOperand(2))->get()) {
+  case ISD::SETOLT:
+  case ISD::SETOLE:
+  case ISD::SETLT:
+  case ISD::SETLE:
+  case ISD::SETULT:
+  case ISD::SETULE: {
+    if (N1FPVal.compare(N2Operand2Val) != APFloat::cmpLessThan)
+      return SDValue();
+
+    RetOpcode = ISD::FMAXNUM;
+    unsigned N2Opcode = N2Node->getOpcode();
+    if (N2Opcode != ISD::FMINNUM && N2Opcode != ISD::FMINNAN)
+      return SDValue();
+  }; break;
+  case ISD::SETOGT:
+  case ISD::SETOGE:
+  case ISD::SETGT:
+  case ISD::SETGE:
+  case ISD::SETUGT:
+  case ISD::SETUGE: {
+    if (N1FPVal.compare(N2Operand2Val) != APFloat::cmpGreaterThan)
+      return SDValue();
+
+    RetOpcode = ISD::FMINNUM;
+    unsigned N2Opcode = N2Node->getOpcode();
+    if (N2Opcode != ISD::FMAXNUM && N2Opcode != ISD::FMAXNAN)
+      return SDValue();
+  }; break;
+  default:
+    return SDValue();
+    break;
+  }
+
+  return DAG.getNode(RetOpcode, SDLoc(N), VT, N2, N1);
+}
+
 SDValue DAGCombiner::visitSELECT(SDNode *N) {
   SDValue N0 = N->getOperand(0);
   SDValue N1 = N->getOperand(1);
   SDValue N2 = N->getOperand(2);
   EVT VT = N->getValueType(0);
   EVT VT0 = N0.getValueType();
   SDLoc DL(N);
 
if (N0.getOpcode() == ISD::SETCC) {
     const TargetOptions &Options = DAG.getTarget().Options;
     if (Options.UnsafeFPMath && VT.isFloatingPoint() && N0.hasOneUse() &&
         DAG.isKnownNeverNaN(N1) && DAG.isKnownNeverNaN(N2)) {
       ISD::CondCode CC = cast<CondCodeSDNode>(N0.getOperand(2))->get();
 
       if (SDValue FMinMax = combineMinNumMaxNum(
               DL, VT, N0.getOperand(0), N0.getOperand(1), N1, N2, CC, TLI, DAG))
         return FMinMax;
+
+      //  t5: i1 = setcc t2, ConstantFP:f1, setgt:ch
+      //  t9: f32 = fmaxnum t2, ConstantFP:f2
+      //  t10: f32 = select t5, ConstantFP:f1, t9
+      // and f1 >= f2
+      // ==> t9  = fmaxnum t2, f2
+      //     t10 = fminnum t9, f1
+      if (isa<ConstantFPSDNode>(N1) && N0->getOpcode() == ISD::SETCC)
+        if (SDValue combinedFP = combineSelectFP(N))
+          return combinedFP;
     }
 
     if ((!LegalOperations &&
          TLI.isOperationLegalOrCustom(ISD::SELECT_CC, VT)) ||
         TLI.isOperationLegal(ISD::SELECT_CC, VT))
       return DAG.getNode(ISD::SELECT_CC, DL, VT, N0.getOperand(0),
                          N0.getOperand(1), N1, N2, N0.getOperand(2));
     return SimplifySelect(DL, N0, N1, N2);

llvm/test/CodeGen/AArch64/aarch64-DAGCombine-fminmax.ll

+; RUN: llc --enable-unsafe-fp-math -mtriple=aarch64-unknown-linux-gnu < %s | FileCheck %s
+; RUN: llc -mtriple=aarch64-unknown-linux-gnu < %s | FileCheck -check-prefix=CHECK1 %s
+
+target datalayout = "e-m:e-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128"
+
+declare float @llvm.minnum.f32(float, float)
+declare float @llvm.maxnum.f32(float, float)
+
+; Function Attrs: norecurse nounwind readnone
+; CHECK:  fmax{{(nm)?}} {{s[0-9]+}}, {{s[0-9]+}}, {{s[0-9]+}}
+; CHECK:  fminnm {{s[0-9]+}}, {{s[0-9]+}}, {{s[0-9]+}}
+; CHECK1:  fmax{{(nm)?}} {{s[0-9]+}}, {{s[0-9]+}}, {{s[0-9]+}}
+; CHECK1-NOT:  fminnm {{s[0-9]+}}, {{s[0-9]+}}, {{s[0-9]+}}
+define float @clampNUM(float %a) local_unnamed_addr #0 {
+entry:
+  %cmp = fcmp ogt float %a, 2.550000e+02
+  %cmp3 = fcmp olt float %a, 1.280000e+02
+  %.a = select i1 %cmp3, float 1.280000e+02, float %a
+  %retval.0 = select i1 %cmp, float 2.550000e+02, float %.a
+  ret float %retval.0
+}
+
+; Function Attrs: norecurse nounwind readnone
+; CHECK:  fminnm {{s[0-9]+}}, {{s[0-9]+}}, {{s[0-9]+}}
+; CHECK:  fmaxnm {{s[0-9]+}}, {{s[0-9]+}}, {{s[0-9]+}}
+; CHECK1:  fminnm {{s[0-9]+}}, {{s[0-9]+}}, {{s[0-9]+}}
+; CHECK1-NOT:  fmaxnm {{s[0-9]+}}, {{s[0-9]+}}, {{s[0-9]+}}
+define float @clampNAN(float %a) local_unnamed_addr #0 {
+entry:
+  %cmp = fcmp olt float %a, 1.280000e+02
+  %cmp2 = fcmp fast ogt float %a, 2.550000e+02
+  %.a = select i1 %cmp2, float 2.550000e+02, float %a
+  %retval.0 = select i1 %cmp, float 1.280000e+02, float %.a
+  ret float %retval.0
+}
+
+; Function Attrs: norecurse nounwind readnone
+; CHECK:  fmaxnm {{s[0-9]+}}, {{s[0-9]+}}, {{s[0-9]+}}
+; CHECK:  fminnm {{s[0-9]+}}, {{s[0-9]+}}, {{s[0-9]+}}
+; CHECK1:  fmaxnm {{s[0-9]+}}, {{s[0-9]+}}, {{s[0-9]+}}
+; CHECK1-NOT:  fminnm {{s[0-9]+}}, {{s[0-9]+}}, {{s[0-9]+}}
+define float @clampIntrinsicFmax(float %a) local_unnamed_addr #0 {
+entry:
+  %cmp = fcmp ogt float %a, 2.550000e+02
+  %.a = call float @llvm.maxnum.f32(float %a, float 1.280000e+02) readnone
+  %retval.0 = select i1 %cmp, float 2.550000e+02, float %.a
+  ret float %retval.0
+}
+
+; Function Attrs: norecurse nounwind readnone
+; CHECK:  fminnm {{s[0-9]+}}, {{s[0-9]+}}, {{s[0-9]+}}
+; CHECK:  fmaxnm {{s[0-9]+}}, {{s[0-9]+}}, {{s[0-9]+}}
+; CHECK1:  fminnm {{s[0-9]+}}, {{s[0-9]+}}, {{s[0-9]+}}
+; CHECK1-NOT:  fmaxnm {{s[0-9]+}}, {{s[0-9]+}}, {{s[0-9]+}}
+define float @clampIntrinsicFmin(float %a) local_unnamed_addr #0 {
+entry:
+  %cmp = fcmp olt float %a, 1.280000e+02
+  %.a = call float @llvm.minnum.f32(float %a, float 2.550000e+02) readnone
+  %retval.0 = select i1 %cmp, float 1.280000e+02, float %.a
+  ret float %retval.0
+}
+
+; Function Attrs: norecurse nounwind readnone
+; CHECK-NOT:  fmin
+; CHECK1-NOT:  fmin
+define double @clampNoConvert(float %a) local_unnamed_addr #0 {
+entry:
+  %cmp = fcmp ogt float %a, 2.550000e+02
+  %.inv = fcmp ole float %a, 3.000000e+00
+  %0 = select i1 %.inv, float 3.000000e+00, float %a
+  %1 = fpext float %0 to double
+  %retval.0 = select i1 %cmp, double 2.550000e+02, double %1
+  ret double %retval.0
+}
+
+; Function Attrs: norecurse nounwind readnone
+; CHECK-NOT:  fmin
+; CHECK1-NOT:  fmin
+define float @clampNo2Convert(float %a) local_unnamed_addr #0 {
+entry:
+  %cmp = fcmp ogt float %a, 2.550000e+02
+  %cmp2 = fcmp fast olt float %a, 0.000000e+00
+  %.a = select i1 %cmp2, float 3.000000e+01, float %a
+  %retval.0 = select i1 %cmp, float 2.550000e+02, float %.a
+  ret float %retval.0
+}
+
+attributes #0 = { norecurse nounwind readnone "no-nans-fp-math"="true" }