DAG: Preserve FMF when creating fminnum/fmaxnum

[arsenm]

Summary

These should preserve the fast math flags on the initial fcmp.
Without this, the nodes may later be expanded to use an
unnecessary quieting operation.

[spatel]

Is this just about 'nnan' behavior? If so, can we just use/expand the existing SelectPatternNaNBehavior?

IOW, why is 'nsz' relevant?
Eli raised a question about 'nsz' on fcmp:
https://bugs.llvm.org/show_bug.cgi?id=38086

Does the motivation for this patch make any of the proposed fixes more or less appealing?

Regardless to any answers to the above, I think we should split this into separate IR/DAG patches with tests for each side.

[arsenm]

In D51646#1224799, @spatel wrote:

Is this just about 'nnan' behavior? If so, can we just use/expand the existing SelectPatternNaNBehavior?

IOW, why is 'nsz' relevant?
Eli raised a question about 'nsz' on fcmp:
https://bugs.llvm.org/show_bug.cgi?id=38086

Does the motivation for this patch make any of the proposed fixes more or less appealing?

Regardless to any answers to the above, I think we should split this into separate IR/DAG patches with tests for each side.

The combine to form minnum/maxnum from cmp/select depends on nsz. The DAG version of the combine checks the global NSZ flag, but I think the direct-from-ir version is broken. I've never really liked the matching here in SelectionDAGBuilder, but haven't looked into what regresses if I try just ripping out the IR matching.

There's also another bug from doing this I've noticed, where the dead nodes are left which breaks hasOneUse optimizations

[arsenm]

lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp

1091–1094

I forgot about this problem. I don't understand why it was trying to strip these flags in the first place, but it will always do it

[arsenm]

Split out DAG part

[aemerson]

lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp

1091–1094

Maybe this is related, we've seen on D51145 that if the FPMathOperator doesn't actually have any FP flags (like insert/extractelements) then the incoming flags result in stripping.

include/llvm/CodeGen/SelectionDAGNodes.h

public:
   /// Default constructor turns off all optimization flags.
   SDNodeFlags()
       : AnyDefined(false), NoUnsignedWrap(false), NoSignedWrap(false),
         Exact(false), NoNaNs(false), NoInfs(false),
         NoSignedZeros(false), AllowReciprocal(false), VectorReduction(false),
         AllowContract(false), ApproximateFuncs(false),
         AllowReassociation(false) {}
 
+  /// Propagate the fast-math-flags from IR FastMathFlags
+  void copyFMF(FastMathFlags FMF) {
+    setNoNaNs(FMF.noNaNs());
+    setNoInfs(FMF.noInfs());
+    setNoSignedZeros(FMF.noSignedZeros());
+    setAllowReciprocal(FMF.allowReciprocal());
+    setAllowContract(FMF.allowContract());
+    setApproximateFuncs(FMF.approxFunc());
+    setAllowReassociation(FMF.allowReassoc());
+  }
+
   /// Propagate the fast-math-flags from an IR FPMathOperator.
   void copyFMF(const FPMathOperator &FPMO) {
-    setNoNaNs(FPMO.hasNoNaNs());
-    setNoInfs(FPMO.hasNoInfs());
-    setNoSignedZeros(FPMO.hasNoSignedZeros());
-    setAllowReciprocal(FPMO.hasAllowReciprocal());
-    setAllowContract(FPMO.hasAllowContract());
-    setApproximateFuncs(FPMO.hasApproxFunc());
-    setAllowReassociation(FPMO.hasAllowReassoc());
+    copyFMF(FPMO.getFastMathFlags());
   }
 
   /// Sets the state of the flags to the defined state.
   void setDefined() { AnyDefined = true; }
   /// Returns true if the flags are in a defined state.
   bool isDefined() const { return AnyDefined; }
 
   // These are mutators for each flag.

lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp

if (auto *FPMO = dyn_cast<FPMathOperator>(&I)) {
     // TODO: We could handle all flags (nsw, etc) here.
     // TODO: If an IR instruction maps to >1 node, only the final node will have
     //       flags set.
     if (SDNode *Node = getNodeForIRValue(&I)) {
       SDNodeFlags IncomingFlags;
       IncomingFlags.copyFMF(*FPMO);
       if (!Node->getFlags().isDefined())
         Node->setFlags(IncomingFlags);
+#if 0
       else
         Node->intersectFlagsWith(IncomingFlags);
+#endif
     }
   }
 
   if (!I.isTerminator() && !HasTailCall &&
       !isStatepoint(&I)) // statepoints handle their exports internally
     CopyToExportRegsIfNeeded(&I);
 
   CurInst = nullptr;
void SelectionDAGBuilder::visitSelect(const User &I) {
 
   SmallVector<SDValue, 4> Values(NumValues);
   SDValue Cond     = getValue(I.getOperand(0));
   SDValue LHSVal   = getValue(I.getOperand(1));
   SDValue RHSVal   = getValue(I.getOperand(2));
   auto BaseOps = {Cond};
   ISD::NodeType OpCode = Cond.getValueType().isVector() ?
     ISD::VSELECT : ISD::SELECT;
+  SDNodeFlags Flags;
 
   // Min/max matching is only viable if all output VTs are the same.
   if (is_splat(ValueVTs)) {
     EVT VT = ValueVTs[0];
     LLVMContext &Ctx = *DAG.getContext();
     auto &TLI = DAG.getTargetLoweringInfo();
 
     // We care about the legality of the operation after it has been type
     // legalized.
     while (TLI.getTypeAction(Ctx, VT) != TargetLoweringBase::TypeLegal &&
            VT != TLI.getTypeToTransformTo(Ctx, VT))
       VT = TLI.getTypeToTransformTo(Ctx, VT);
 
     // If the vselect is legal, assume we want to leave this as a vector setcc +
     // vselect. Otherwise, if this is going to be scalarized, we want to see if
     // min/max is legal on the scalar type.
     bool UseScalarMinMax = VT.isVector() &&
       !TLI.isOperationLegalOrCustom(ISD::VSELECT, VT);
 
     Value *LHS, *RHS;
     auto SPR = matchSelectPattern(const_cast<User*>(&I), LHS, RHS);
+    Flags.copyFMF(SPR.FMF);
+
     ISD::NodeType Opc = ISD::DELETED_NODE;
     switch (SPR.Flavor) {
     case SPF_UMAX:    Opc = ISD::UMAX; break;
     case SPF_UMIN:    Opc = ISD::UMIN; break;
     case SPF_SMAX:    Opc = ISD::SMAX; break;
     case SPF_SMIN:    Opc = ISD::SMIN; break;
     case SPF_FMINNUM:
       switch (SPR.NaNBehavior) {
void SelectionDAGBuilder::visitSelect(const User &I) {
   }
 
   for (unsigned i = 0; i != NumValues; ++i) {
     SmallVector<SDValue, 3> Ops(BaseOps.begin(), BaseOps.end());
     Ops.push_back(SDValue(LHSVal.getNode(), LHSVal.getResNo() + i));
     Ops.push_back(SDValue(RHSVal.getNode(), RHSVal.getResNo() + i));
     Values[i] = DAG.getNode(OpCode, getCurSDLoc(),
                             LHSVal.getNode()->getValueType(LHSVal.getResNo()+i),
-                            Ops);
+                            Ops, Flags);
   }
 
   setValue(&I, DAG.getNode(ISD::MERGE_VALUES, getCurSDLoc(),
                            DAG.getVTList(ValueVTs), Values));
 }
 
 void SelectionDAGBuilder::visitTrunc(const User &I) {
   // TruncInst cannot be a no-op cast because sizeof(src) > sizeof(dest).