[SelectionDAG] propagate 'afn' and 'reassoc' from IR fast-math-flags

[spatel]

Summary

This is extracted/simplified from D45710 because I think we should break that into smaller pieces and have tests for each step. If nobody else thinks that's necessary, I'll abandon.

Note that I'm proposing that we go without adding an 'isFast()' umbrella in the DAG to avoid that misuse entirely. This means that the relatively few current folds that use SDNodeFlags may have bugs where we are overstepping the bounds of 'reassoc' alone.

But that's ok because these folds are also checking TargetOptions.UnsafeFPMath which isn't actually an umbrella flag based on its code comment:

/// UnsafeFPMath - This flag is enabled when the
/// -enable-unsafe-fp-math flag is specified on the command line.  When
/// this flag is off (the default), the code generator is not allowed to
/// produce results that are "less precise" than IEEE allows.  This includes
/// use of X86 instructions like FSIN and FCOS instead of libcalls.

I think this is intended to have the same meaning as 'reassoc' in IR:

reassoc
  Allow reassociation transformations for floating-point instructions. This may dramatically change results in floating-point.

So this flag can be set independently of NoSignedZerosFPMath for example, and we're supposed to be restricting folds based on that possibility. If not, then we already have a bug (most likely with handling of -0.0).

To fix that, we need to audit each transform (as we've been doing in IR) to see exactly which FMF are required for each transform.

So in summary, this patch is intended to be no more wrong than we already may be. It just seeks to unify the flags between IR and the DAG.

[wristow]

I realize that you're abandoning this, in favor of simplifying D45710, but there was one point here that I wanted to comment on. Specifically:

... This means that the relatively few current folds that use SDNodeFlags may have bugs where we are overstepping the bounds of 'reassoc' alone.

But that's ok because these folds are also checking TargetOptions.UnsafeFPMath which isn't actually an umbrella flag based on its code comment:

/// UnsafeFPMath - This flag is enabled when the
/// -enable-unsafe-fp-math flag is specified on the command line.  When
/// this flag is off (the default), the code generator is not allowed to
/// produce results that are "less precise" than IEEE allows.  This includes
/// use of X86 instructions like FSIN and FCOS instead of libcalls.

I think this is intended to have the same meaning as 'reassoc' in IR:
...

In practice, with the Clang front-end, this flag does mean more than just the 'reassoc' flag. In particular with Clang, '-enable-unsafe-fp-math' is specified on the cc1 command-line when the following user-level switches are specified:

-fno-math-errno -fassociative-math -freciprocal-math -fno-signed-zeros -fno-trapping-math

'reassoc' in IR can be true when (for example) allow-reciprocal ('arcp') is false. So (again, with the Clang front-end), the following command (which does not pass '-enable-unsafe-fp-math'):

-fno-math-errno -fassociative-math -fno-signed-zeros -fno-trapping-math

will set 'reassoc'.

So if I've understood this correctly, regarding:

So in summary, this patch is intended to be no more wrong than we already may be. It just seeks to unify the flags between IR and the DAG.

it's theoretically possible for this patch to be more aggressive (and so "be more wrong"). In practice, I don't think it will do harm because of the way these flags are currently used.

[spatel]

In D46322#1086104, @wristow wrote:

In practice, with the Clang front-end, this flag does mean more than just the 'reassoc' flag. In particular with Clang, '-enable-unsafe-fp-math' is specified on the cc1 command-line when the following user-level switches are specified:

-fno-math-errno -fassociative-math -freciprocal-math -fno-signed-zeros -fno-trapping-math

'reassoc' in IR can be true when (for example) allow-reciprocal ('arcp') is false. So (again, with the Clang front-end), the following command (which does not pass '-enable-unsafe-fp-math'):

-fno-math-errno -fassociative-math -fno-signed-zeros -fno-trapping-math

will set 'reassoc'.

Good point. I see this as even more reason to break up the steps of propagating/using the flags in the DAG and MI as much as possible and add tests along the way, so we have a better understanding of exactly what FMF semantics are in play. IOW, reduce the risk that we'll introduce mistakes as we update the plumbing.

include/llvm/CodeGen/SelectionDAGNodes.h

private:
   // This bit is used to determine if the flags are in a defined state.
   // Flag bits can only be masked out during intersection if the masking flags
   // are defined.
   bool AnyDefined : 1;
 
   bool NoUnsignedWrap : 1;
   bool NoSignedWrap : 1;
   bool Exact : 1;
-  bool UnsafeAlgebra : 1;
   bool NoNaNs : 1;
   bool NoInfs : 1;
   bool NoSignedZeros : 1;
   bool AllowReciprocal : 1;
   bool VectorReduction : 1;
   bool AllowContract : 1;
+  bool ApproximateFuncs : 1;
+  bool AllowReassociation : 1;
 
 public:
   /// Default constructor turns off all optimization flags.
   SDNodeFlags()
       : AnyDefined(false), NoUnsignedWrap(false), NoSignedWrap(false),
-        Exact(false), UnsafeAlgebra(false), NoNaNs(false), NoInfs(false),
-        NoSignedZeros(false), AllowReciprocal(false), VectorReduction(false),
-        AllowContract(false) {}
+        Exact(false), NoNaNs(false), NoInfs(false), NoSignedZeros(false),
+        AllowReciprocal(false), VectorReduction(false), AllowContract(false),
+        ApproximateFuncs(false), AllowReassociation(false) {}
 
   /// 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.
   void setNoUnsignedWrap(bool b) {
     setDefined();
     NoUnsignedWrap = b;
   }
   void setNoSignedWrap(bool b) {
     setDefined();
     NoSignedWrap = b;
   }
   void setExact(bool b) {
     setDefined();
     Exact = b;
   }
-  void setUnsafeAlgebra(bool b) {
-    setDefined();
-    UnsafeAlgebra = b;
-  }
   void setNoNaNs(bool b) {
     setDefined();
     NoNaNs = b;
   }
   void setNoInfs(bool b) {
     setDefined();
     NoInfs = b;
   }
   void setNoSignedZeros(bool b) {
     setDefined();
     NoSignedZeros = b;
   }
   void setAllowReciprocal(bool b) {
     setDefined();
     AllowReciprocal = b;
   }
   void setVectorReduction(bool b) {
     setDefined();
     VectorReduction = b;
   }
   void setAllowContract(bool b) {
     setDefined();
     AllowContract = b;
   }
+  void setApproximateFuncs(bool b) {
+    setDefined();
+    ApproximateFuncs = b;
+  }
+  void setAllowReassociation(bool b) {
+    setDefined();
+    AllowReassociation = b;
+  }
 
   // These are accessors for each flag.
   bool hasNoUnsignedWrap() const { return NoUnsignedWrap; }
   bool hasNoSignedWrap() const { return NoSignedWrap; }
   bool hasExact() const { return Exact; }
-  bool hasUnsafeAlgebra() const { return UnsafeAlgebra; }
   bool hasNoNaNs() const { return NoNaNs; }
   bool hasNoInfs() const { return NoInfs; }
   bool hasNoSignedZeros() const { return NoSignedZeros; }
   bool hasAllowReciprocal() const { return AllowReciprocal; }
   bool hasVectorReduction() const { return VectorReduction; }
   bool hasAllowContract() const { return AllowContract; }
+  bool hasApproximateFuncs() const { return ApproximateFuncs; }
+  bool hasAllowReassociation() const { return AllowReassociation; }
 
   /// Clear any flags in this flag set that aren't also set in Flags.
-  /// If the given Flags are undefined then don't do anything.
+  /// If the given Flags are undefined, then don't do anything.
   void intersectWith(const SDNodeFlags Flags) {
     if (!Flags.isDefined())
       return;
     NoUnsignedWrap &= Flags.NoUnsignedWrap;
     NoSignedWrap &= Flags.NoSignedWrap;
     Exact &= Flags.Exact;
-    UnsafeAlgebra &= Flags.UnsafeAlgebra;
     NoNaNs &= Flags.NoNaNs;
     NoInfs &= Flags.NoInfs;
     NoSignedZeros &= Flags.NoSignedZeros;
     AllowReciprocal &= Flags.AllowReciprocal;
     VectorReduction &= Flags.VectorReduction;
     AllowContract &= Flags.AllowContract;
+    ApproximateFuncs &= Flags.ApproximateFuncs;
+    AllowReassociation &= Flags.AllowReassociation;
   }
 };
 
 /// Represents one node in the SelectionDAG.
 ///
 class SDNode : public FoldingSetNode, public ilist_node<SDNode> {
 private:
   /// The operation that this node performs.

lib/CodeGen/SelectionDAG/DAGCombiner.cpp

if (DAG.getDataLayout().isBigEndian())
       std::reverse(Ops.end()-NumOutputsPerInput, Ops.end());
   }
 
   return DAG.getBuildVector(VT, DL, Ops);
 }
 
 static bool isContractable(SDNode *N) {
   SDNodeFlags F = N->getFlags();
-  return F.hasAllowContract() || F.hasUnsafeAlgebra();
+  return F.hasAllowContract() || F.hasAllowReassociation();
 }
 
 /// Try to perform FMA combining on a given FADD node.
 SDValue DAGCombiner::visitFADDForFMACombine(SDNode *N) {
   SDValue N0 = N->getOperand(0);
   SDValue N1 = N->getOperand(1);
   EVT VT = N->getValueType(0);
   SDLoc SL(N);
if (N1CFP && N1CFP->isExactlyValue(1.0))
     return DAG.getNode(ISD::FADD, SDLoc(N), VT, N0, N2);
 
   // Canonicalize (fma c, x, y) -> (fma x, c, y)
   if (isConstantFPBuildVectorOrConstantFP(N0) &&
      !isConstantFPBuildVectorOrConstantFP(N1))
     return DAG.getNode(ISD::FMA, SDLoc(N), VT, N1, N0, N2);
 
   // TODO: FMA nodes should have flags that propagate to the created nodes.
-  // For now, create a Flags object for use with all unsafe math transforms.
+  // For now, create a Flags object for use with reassociation math transforms.
   SDNodeFlags Flags;
-  Flags.setUnsafeAlgebra(true);
+  Flags.setAllowReassociation(true);
 
   if (Options.UnsafeFPMath) {
     // (fma x, c1, (fmul x, c2)) -> (fmul x, c1+c2)
     if (N2.getOpcode() == ISD::FMUL && N0 == N2.getOperand(0) &&
         isConstantFPBuildVectorOrConstantFP(N1) &&
         isConstantFPBuildVectorOrConstantFP(N2.getOperand(1))) {
       return DAG.getNode(ISD::FMUL, DL, VT, N0,
                          DAG.getNode(ISD::FADD, DL, VT, N1, N2.getOperand(1),
SDValue DAGCombiner::visitFSQRT(SDNode *N) {
   if (!DAG.getTarget().Options.UnsafeFPMath)
     return SDValue();
 
   SDValue N0 = N->getOperand(0);
   if (TLI.isFsqrtCheap(N0, DAG))
     return SDValue();
 
   // TODO: FSQRT nodes should have flags that propagate to the created nodes.
-  // For now, create a Flags object for use with all unsafe math transforms.
+  // For now, create a Flags object for use with reassociation math transforms.
   SDNodeFlags Flags;
-  Flags.setUnsafeAlgebra(true);
+  Flags.setAllowReassociation(true);
   return buildSqrtEstimate(N0, Flags);
 }
 
 /// copysign(x, fp_extend(y)) -> copysign(x, y)
 /// copysign(x, fp_round(y)) -> copysign(x, y)
 static inline bool CanCombineFCOPYSIGN_EXTEND_ROUND(SDNode *N) {
   SDValue N1 = N->getOperand(1);
   if ((N1.getOpcode() == ISD::FP_EXTEND ||

lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp

void SelectionDAGBuilder::visitBinary(const User &I, unsigned OpCode) {
   Flags.setNoSignedWrap(nsw);
   Flags.setNoUnsignedWrap(nuw);
   Flags.setVectorReduction(vec_redux);
   Flags.setAllowReciprocal(FMF.allowReciprocal());
   Flags.setAllowContract(FMF.allowContract());
   Flags.setNoInfs(FMF.noInfs());
   Flags.setNoNaNs(FMF.noNaNs());
   Flags.setNoSignedZeros(FMF.noSignedZeros());
-  Flags.setUnsafeAlgebra(FMF.isFast());
+  Flags.setApproximateFuncs(FMF.approxFunc());
+  Flags.setAllowReassociation(FMF.allowReassoc());
 
   SDValue BinNodeValue = DAG.getNode(OpCode, getCurSDLoc(), Op1.getValueType(),
                                      Op1, Op2, Flags);
   setValue(&I, BinNodeValue);
 }
 
 void SelectionDAGBuilder::visitShift(const User &I, unsigned Opcode) {
   SDValue Op1 = getValue(I.getOperand(0));

lib/CodeGen/SelectionDAG/SelectionDAGDumper.cpp

if (getFlags().hasNoUnsignedWrap())
     OS << " nuw";
 
   if (getFlags().hasNoSignedWrap())
     OS << " nsw";
 
   if (getFlags().hasExact())
     OS << " exact";
 
-  if (getFlags().hasUnsafeAlgebra())
-    OS << " unsafe";
-
   if (getFlags().hasNoNaNs())
     OS << " nnan";
 
   if (getFlags().hasNoInfs())
     OS << " ninf";
 
   if (getFlags().hasNoSignedZeros())
     OS << " nsz";
 
   if (getFlags().hasAllowReciprocal())
     OS << " arcp";
 
   if (getFlags().hasAllowContract())
     OS << " contract";
 
+  if (getFlags().hasApproximateFuncs())
+    OS << " afn";
+
+  if (getFlags().hasAllowReassociation())
+    OS << " reassoc";
+
   if (getFlags().hasVectorReduction())
     OS << " vector-reduction";
 
   if (const MachineSDNode *MN = dyn_cast<MachineSDNode>(this)) {
     if (!MN->memoperands_empty()) {
       OS << "<";
       OS << "Mem:";
       for (MachineSDNode::mmo_iterator i = MN->memoperands_begin(),

lib/Target/AArch64/AArch64ISelLowering.cpp

SDValue AArch64TargetLowering::getSqrtEstimate(SDValue Operand,
   if (Enabled == ReciprocalEstimate::Enabled ||
       (Enabled == ReciprocalEstimate::Unspecified && Subtarget->useRSqrt()))
     if (SDValue Estimate = getEstimate(Subtarget, AArch64ISD::FRSQRTE, Operand,
                                        DAG, ExtraSteps)) {
       SDLoc DL(Operand);
       EVT VT = Operand.getValueType();
 
       SDNodeFlags Flags;
-      Flags.setUnsafeAlgebra(true);
+      Flags.setAllowReassociation(true);
 
       // Newton reciprocal square root iteration: E * 0.5 * (3 - X * E^2)
       // AArch64 reciprocal square root iteration instruction: 0.5 * (3 - M * N)
       for (int i = ExtraSteps; i > 0; --i) {
         SDValue Step = DAG.getNode(ISD::FMUL, DL, VT, Estimate, Estimate,
                                    Flags);
         Step = DAG.getNode(AArch64ISD::FRSQRTS, DL, VT, Operand, Step, Flags);
         Estimate = DAG.getNode(ISD::FMUL, DL, VT, Estimate, Step, Flags);
SDValue AArch64TargetLowering::getRecipEstimate(SDValue Operand,
                                                 int &ExtraSteps) const {
   if (Enabled == ReciprocalEstimate::Enabled)
     if (SDValue Estimate = getEstimate(Subtarget, AArch64ISD::FRECPE, Operand,
                                        DAG, ExtraSteps)) {
       SDLoc DL(Operand);
       EVT VT = Operand.getValueType();
 
       SDNodeFlags Flags;
-      Flags.setUnsafeAlgebra(true);
+      Flags.setAllowReassociation(true);
 
       // Newton reciprocal iteration: E * (2 - X * E)
       // AArch64 reciprocal iteration instruction: (2 - M * N)
       for (int i = ExtraSteps; i > 0; --i) {
         SDValue Step = DAG.getNode(AArch64ISD::FRECPS, DL, VT, Operand,
                                    Estimate, Flags);
         Estimate = DAG.getNode(ISD::FMUL, DL, VT, Estimate, Step, Flags);
       }

lib/Target/AMDGPU/SIISelLowering.cpp

 SDValue SITargetLowering::lowerFastUnsafeFDIV(SDValue Op,
                                               SelectionDAG &DAG) const {
   SDLoc SL(Op);
   SDValue LHS = Op.getOperand(0);
   SDValue RHS = Op.getOperand(1);
   EVT VT = Op.getValueType();
   const SDNodeFlags Flags = Op->getFlags();
   bool Unsafe = DAG.getTarget().Options.UnsafeFPMath ||
-                Flags.hasUnsafeAlgebra() || Flags.hasAllowReciprocal();
+                Flags.hasAllowReassociation() || Flags.hasAllowReciprocal();
 
   if (!Unsafe && VT == MVT::f32 && Subtarget->hasFP32Denormals())
     return SDValue();
 
   if (const ConstantFPSDNode *CLHS = dyn_cast<ConstantFPSDNode>(LHS)) {
     if (Unsafe || VT == MVT::f32 || VT == MVT::f16) {
       if (CLHS->isExactlyValue(1.0)) {
         // v_rcp_f32 and v_rsq_f32 do not support denormals, and according to
unsigned SITargetLowering::getFusedOpcode(const SelectionDAG &DAG,
   // Only do this if we are not trying to support denormals. v_mad_f32 does not
   // support denormals ever.
   if ((VT == MVT::f32 && !Subtarget->hasFP32Denormals()) ||
       (VT == MVT::f16 && !Subtarget->hasFP16Denormals()))
     return ISD::FMAD;
 
   const TargetOptions &Options = DAG.getTarget().Options;
   if ((Options.AllowFPOpFusion == FPOpFusion::Fast || Options.UnsafeFPMath ||
-       (N0->getFlags().hasUnsafeAlgebra() &&
-        N1->getFlags().hasUnsafeAlgebra())) &&
+       (N0->getFlags().hasAllowReassociation() &&
+        N1->getFlags().hasAllowReassociation())) &&
       isFMAFasterThanFMulAndFAdd(VT)) {
     return ISD::FMA;
   }
 
   return 0;
 }
 
 static SDValue getMad64_32(SelectionDAG &DAG, const SDLoc &SL,

test/CodeGen/X86/fmf-propagation.ll

 ; REQUIRES: asserts
 ; RUN: llc -mtriple=x86_64-unknown-unknown -debug-only=isel < %s -o /dev/null 2>&1 | FileCheck %s
 
 ; This tests the propagation of fast-math-flags from IR instructions to SDNodeFlags.
 
-; FIXME: 'afn' and 'reassoc' were dropped. With 'fast', 'reassoc' got renamed to 'unsafe'.
-
 ; CHECK-LABEL: Initial selection DAG: %bb.0 'fmf_transfer:'
 
 ; CHECK:         t5: f32 = fadd nsz t2, t4
 ; CHECK-NEXT:    t6: f32 = fadd arcp t5, t4
 ; CHECK-NEXT:    t7: f32 = fadd nnan t6, t4
 ; CHECK-NEXT:    t8: f32 = fadd ninf t7, t4
 ; CHECK-NEXT:    t9: f32 = fadd contract t8, t4
-; CHECK-NEXT:    t10: f32 = fadd t9, t4
-; CHECK-NEXT:    t11: f32 = fadd t10, t4
-; CHECK-NEXT:    t12: f32 = fadd unsafe nnan ninf nsz arcp contract t11, t4
+; CHECK-NEXT:    t10: f32 = fadd afn t9, t4
+; CHECK-NEXT:    t11: f32 = fadd reassoc t10, t4
+; CHECK-NEXT:    t12: f32 = fadd nnan ninf nsz arcp contract afn reassoc t11, t4
 
 ; CHECK: Optimized lowered selection DAG: %bb.0 'fmf_transfer:'
 
 define float @fmf_transfer(float %x, float %y) {
   %f1 = fadd nsz float %x, %y
   %f2 = fadd arcp float %f1, %y
   %f3 = fadd nnan float %f2, %y
   %f4 = fadd ninf float %f3, %y
   %f5 = fadd contract float %f4, %y
   %f6 = fadd afn float %f5, %y
   %f7 = fadd reassoc float %f6, %y
   %f8 = fadd fast float %f7, %y
   ret float %f8
 }