Fast Math Flag mapping into SDNode

[mcberg2017]

Summary

Adding support for Fast flags in the SDNode to leverage fast math sub flag usage.

[spatel]

Thanks for working on this. Adding some more potential reviewers...

If I'm seeing it correctly, there are several independent pieces to this patch, so please split it up as much as possible and have tests for each part.

As a possible first step, can you compare the FMF propagation from IR to SDAG to the proposal in D37686 (@jbhateja are you planning to proceed on that patch?)

[mcberg2017]

This upload contains some additional tests as requested. I did some digging to find coverage for Aggressive fma. That is also in the test path now. As only the machine instructions code is separable and also small it seems better to leave it all in one place for context. I did look at the cross reference code in D37686. There is some similar coverage with the initial code and test changes and some that is outside those that coverage for different reasons.

[arsenm]

include/llvm/CodeGen/MachineInstr.h

76–98 ↗

(On Diff #142879)

These should be a separate patch from the DAG changes, and include the necessary MIR printer/parser changes to handle them

106 ↗

(On Diff #142879)

If this increases the size, can we decrease this to uint16?

110 ↗

(On Diff #142879)

Does this change the overall size of the MachineInstr struct, or is there already enough padding to use?

lib/CodeGen/MachineInstr.cpp

536–539 ↗

(On Diff #142879)

I thought we were moving away from an overly generic "fast" terminology. This can also go in the header

lib/CodeGen/SelectionDAG/DAGCombiner.cpp

681

I think any of these DAG changes are a separate patch from the MI changes

[spatel]

include/llvm/CodeGen/SelectionDAGNodes.h

361–362

This is where *this patch* should start and end. Why do we still need an UnsafeAlgebra bit after adding AllowReassociation?

lib/CodeGen/MachineInstr.cpp

536–539 ↗

(On Diff #142879)

That's correct. In IR, we're replacing uses of isFast() with the subset of FMF that are actually required for each transform.

There's no transform that actually requires every flag, so introducing this API for MI is inviting trouble. Having isFast() in the DAG as a bridge is fine, but the goal should be to refine the DAG transforms.

[mcberg2017]

include/llvm/CodeGen/MachineInstr.h

110 ↗

(On Diff #142879)

When I put up the new review with InstrEmitter.cpp and MachineInstr.h (as the cpp file will no longer have changes), I will answer the size question for Flags there.

[mcberg2017]

I have split off the MachineInstr based code for a separate review as requested, will provide a link once that is up.

[mcberg2017]

There is a related review for the MachineInstr code in D45781.

[mcberg2017]

include/llvm/CodeGen/MachineInstr.h

110 ↗

(On Diff #142879)

The padding in theMIFlag enum is such that the new flags do not change the size of the class, however, the Flags field changes it by 1 byte in size.

[spatel]

include/llvm/CodeGen/SelectionDAGNodes.h

361–362

This is marked as 'Done', but UnsafeAlgebra is still here without comment?

[mcberg2017]

include/llvm/CodeGen/SelectionDAGNodes.h

361–362

It still present as there has not yet been any cleanup for AMDGPU, looputils and vectorization with regard to hasUnsafeAlgebra which tests that use the value. Also Aarch64 still sets it but it is not used in get context. I would like to defer this issue to cleanup work after the fact if that is ok. I will add a comment on the field for now that it is still in use under the contexts above.

[wristow]

include/llvm/CodeGen/SelectionDAGNodes.h

361–362

From my POV, I think the adding of 'AllowReassociation' and 'ApproximateFuncs' goes hand-in-hand with the cleanup of the umbrella aspect of 'UnsafeAlgebra' (that is, with the removal of the 'UnsafeAlgebra' flag). I have a bias against the umbrella aspect of 'UnsafeAlgebra', so I tend to see that cleanup as a good first step (and I think that cleanup will be straightforward). But as long as there is a comment explaining the intention of cleaning it up, and as long as the deferring of the cleanup is intended to be for a very short time, that's OK with me.

[spatel]

include/llvm/CodeGen/SelectionDAGNodes.h

361–362

+1. Sorry if it wasn't clear when I said this is where the first patch should end. Warren's description is what I was trying to suggest. Unless I'm missing something, this is a simple first step that just involves some renaming (no functional changes), eg:

Flags.setUnsafeAlgebra(FMF.isFast());

I think you mention looputils and vectorization because a simple grep turns up 'unsafealgebra' there, but that's not based on this bit - those are IR synonyms that should be fixed independently to the updated vocabulary (and then fixed properly to the correct FMF requirements):

bool hasUnsafeAlgebra() {
  return InductionBinOp && !cast<FPMathOperator>(InductionBinOp)->isFast();

There should be a very small number of DAG changes to remove/rename/replace the DAG uses of getFlags().hasUnsafeAlgebra() -> getFlags.isFast().

So I'd prefer to do that as a preliminary cleanup because DAG FMF is already too messy.

[mcberg2017]

Please see the changes related to hasUnsafe... and setUnsafe... wrt the removal of UnsafeAlgebra from SDNodeFlags. I believe this was the requested change. All testing passes as before.

[mcberg2017]

Stay tuned, more changes coming to both reviews. There will be new upload shortly...

[mcberg2017]

When writing and testing the mir fmf test, I found I needed to augment interior fma flags as well and the InstrEmitter.cpp change is here now.

[mcberg2017]

Updated after testing with changes for AMDGPU and AArch64 with a test: All updated from D46322.
AMDGPU may need further testing on proprietary backends.

[mcberg2017]

I have greatly reduced the size and scope of this review, other reviews will follow with the initial information, after the fact. This patch is now independent as stand alone.

[spatel]

I discussed this and D46322 with Michael offline, so I think we're not that far off now from a first step. I'll abandon the other patch.

But I'm still confused - why are the changes to SelectionDAGBuilder::visitTargetIntrinsic(), SelectionDAGBuilder::visitIntrinsicCall() and the DAGCombiner diffs to support those new propagations of FMF in this patch? I'd like to shrink this more, so we're just getting the FMF bits to line up.

include/llvm/CodeGen/SelectionDAGNodes.h

368

Typo: "ApproximateFuncs"

938–943

Flags.isFast() ?

[mcberg2017]

Reduced some more to only replacing unsafe in required context, and in those using the flags we have since we now have the ability while proffering SDNode flag context for all the fast sub flags and keeping isFast. We can make case by case adjustments to flag usage after the fact.

[mcberg2017]

lib/CodeGen/SelectionDAG/DAGCombiner.cpp

9545

FMA combining can happen without reasssocation, we should just keep this to contract.

10573

Since we have the flags lets use them.

10847

Since we have the flags, lets use them.

lib/Target/AMDGPU/SIISelLowering.cpp

5350

Question: unsafe implies arcp or it should, should reassocation imply that too or should we go without reassociation in this case?

[wristow]

This is moving closer to being an NFC patch that purely cleans up the FMF bits, which makes sense to me. Ideally, it would be nice if only that clean-up was done in the initial patch, and then a later patch makes changes to DAGCombiner, etc. I think that's what Sanjay is saying. (But I think that because the semantics of the 'UnsafeAlgebra' flag (which is being removed and references being replaced by something else) aren't precisely defined, it may not be a guaranteed NFC patch -- but that's the goal.)

Along these same lines, the changes in the two SelectionDAG::getNode() methods in SelectionDAG.cpp seem like they should be deferred to a later patch.

[spatel]

lib/CodeGen/SelectionDAG/DAGCombiner.cpp

9545

This is more restrictive than it should be. If something is reassociable, then doesn't that imply that it is also contractable?

10573

I don't think this is working as you intended. The FMA node probably doesn't have any FMF set on its node...because we're not propagating that from IR yet.

I think we should leave this as-is for the moment - just change setUnsafeAlgebra() to setAllowReassociation(). I've added tests that will hopefully demonstrate the functional difference of this change at:
rL331471
rL331476

lib/Target/AMDGPU/SIISelLowering.cpp

5350

Reciprocal transforms are a special case of reassociation, but not enabled by 'reassoc' alone. Ie, the reason 'arcp' is its own bit is because there are users that want to enable/disable this particular transform independently of other algebraic transforms.

So in this case, I think we want to just remove the hasUnsafeAlgebra() check because that (isFast) is implied by hasAllowReciprocal():
bool Unsafe = DAG.getTarget().Options.UnsafeFPMath || Flags.hasAllowReciprocal();

But @arsenm or someone with AMDGPU knowledge should confirm if that is the intent here. The sqrt transform might want to use 'afn' now that we're propagating it, but that might be an independent change too. It's a mess. :)

[mcberg2017]

With only binary ops reflecting flags, our hands are kind of tied, so this should suffice until we have flag propagation in place.

[mcberg2017]

Updated for a newly added test

[spatel]

LGTM.

We're down to the minimal patch now, so the AMDGPU change is actually NFC - the 'unsafe' node flag on an fdiv could only be set when all IR FMF were set (aka isFast). That means 'AllowReciprocal' must also be set.
Let me cc a few more potential AMDGPU folks, so they're aware of this change: @nhaehnle @rampitec @FarhanaAleen @tstellar

[spatel]

This review should have included the llvm-commits list as a subscriber.

[rampitec]

lib/Target/AMDGPU/SIISelLowering.cpp

6700–6701

Production of a fused fma opcode is not tied to reassociation. The question here is legality of intermediate result rounding.

[mcberg2017]

lib/Target/AMDGPU/SIISelLowering.cpp

6700–6701

Stanislav, what change if any do you request then for this code?

[rampitec]

lib/Target/AMDGPU/SIISelLowering.cpp

6700–6701

I would say AllowContract can be added here, but reassociation is not a sufficient or required condition.

[mcberg2017]

lib/Target/AMDGPU/SIISelLowering.cpp

6700–6701

Ok, will do.

[spatel]

lib/Target/AMDGPU/SIISelLowering.cpp

6700–6701

So this is an existing bug in the code? It shouldn't be checking hasUnsafeAlgebra (or Options.UnsafeFPMath). Can we add a test for that?

[rampitec]

lib/Target/AMDGPU/SIISelLowering.cpp

6700–6701

I do not think it is a bug, this is legal to do with unsafe algebra.
At the end of the day I think if you remove reassociation from the condition and add AllowContract check it should then boil down to the question if existing lit tests will pass.

[mcberg2017]

lib/Target/AMDGPU/SIISelLowering.cpp

6700–6701

With the change to hasAllowContract, existing lit tests all pass.

[rampitec]

lib/Target/AMDGPU/SIISelLowering.cpp

6700–6701

Thanks. I think we are fine then.

[mcberg2017]

Updated with change to AMD fma case.

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),
+        Exact(false), NoNaNs(false), NoInfs(false),
         NoSignedZeros(false), AllowReciprocal(false), VectorReduction(false),
-        AllowContract(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; }
+
+  bool isFast() const {
+    return NoSignedZeros && AllowReciprocal && NoNaNs && NoInfs &&
+           AllowContract && ApproximateFuncs && 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.
   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.
SDNode *getGluedUser() const {
     for (use_iterator UI = use_begin(), UE = use_end(); UI != UE; ++UI)
       if (UI.getUse().get().getValueType() == MVT::Glue)
         return *UI;
     return nullptr;
   }
 
   const SDNodeFlags getFlags() const { return Flags; }
   void setFlags(SDNodeFlags NewFlags) { Flags = NewFlags; }
+  bool isFast() { return Flags.isFast(); }
 
   /// Clear any flags in this node that aren't also set in Flags.
   /// If Flags is not in a defined state then this has no effect.
   void intersectFlagsWith(const SDNodeFlags Flags);
 
   /// Return the number of values defined/returned by this operator.
   unsigned getNumValues() const { return NumValues; }
 
   /// Return the type of a specified result.
   EVT getValueType(unsigned ResNo) const {
     assert(ResNo < NumValues && "Illegal result number!");
     return ValueList[ResNo];
   }

lib/CodeGen/SelectionDAG/DAGCombiner.cpp

static char isNegatibleForFree(SDValue Op, bool LegalOperations,
   if (!Op.hasOneUse())
     if (!(Op.getOpcode() == ISD::FP_EXTEND &&
           TLI.isFPExtFree(VT, Op.getOperand(0).getValueType())))
       return 0;
 
   // Don't recurse exponentially.
   if (Depth > 6) return 0;
 
   switch (Op.getOpcode()) {
   default: return false;
   case ISD::ConstantFP: {
     if (!LegalOperations)
       return 1;
 
     // Don't invert constant FP values after legalization unless the target says
     // the negated constant is legal.
     return TLI.isOperationLegal(ISD::ConstantFP, VT) ||
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),
                                      Flags), Flags);
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 ||
        N1.getOpcode() == ISD::FP_ROUND)) {

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

 // instructions.
 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();
+  bool Unsafe = DAG.getTarget().Options.UnsafeFPMath || 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().hasAllowContract() &&
+        N1->getFlags().hasAllowContract())) &&
       isFMAFasterThanFMulAndFAdd(VT)) {
     return ISD::FMA;
   }
 
   return 0;
 }
 
 static SDValue getMad64_32(SelectionDAG &DAG, const SDLoc &SL,

test/CodeGen/PowerPC/fmf-propagation.ll

; GLOBAL-NEXT:    blr
   %add = fadd contract float %mul, %z
   ret float %add
 }
 
 ; Reassociation implies that FMA contraction is allowed.
 
 ; FMFDEBUG-LABEL: Optimized lowered selection DAG: %bb.0 'fmul_fadd_reassoc1:'
 ; FMFDEBUG:         fmul {{t[0-9]+}}, {{t[0-9]+}}
-; FMFDEBUG:         fadd {{t[0-9]+}}, {{t[0-9]+}}
+; FMFDEBUG:         fadd reassoc {{t[0-9]+}}, {{t[0-9]+}}
 ; FMFDEBUG:       Type-legalized selection DAG: %bb.0 'fmul_fadd_reassoc1:'
 
 define float @fmul_fadd_reassoc1(float %x, float %y, float %z) {
 ; FMF-LABEL: fmul_fadd_reassoc1:
 ; FMF:       # %bb.0:
 ; FMF-NEXT:    xsmulsp 0, 1, 2
 ; FMF-NEXT:    xsaddsp 1, 0, 3
 ; FMF-NEXT:    blr
 ;
 ; GLOBAL-LABEL: fmul_fadd_reassoc1:
 ; GLOBAL:       # %bb.0:
 ; GLOBAL-NEXT:    xsmaddasp 3, 1, 2
 ; GLOBAL-NEXT:    fmr 1, 3
 ; GLOBAL-NEXT:    blr
   %mul = fmul float %x, %y
   %add = fadd reassoc float %mul, %z
   ret float %add
 }
 
 ; This shouldn't change anything - the intermediate fmul result is now also flagged.
 
 ; FMFDEBUG-LABEL: Optimized lowered selection DAG: %bb.0 'fmul_fadd_reassoc2:'
-; FMFDEBUG:         fmul {{t[0-9]+}}, {{t[0-9]+}}
-; FMFDEBUG:         fadd {{t[0-9]+}}, {{t[0-9]+}}
+; FMFDEBUG:         fma {{t[0-9]+}}, {{t[0-9]+}}
 ; FMFDEBUG:       Type-legalized selection DAG: %bb.0 'fmul_fadd_reassoc2:'
 
 define float @fmul_fadd_reassoc2(float %x, float %y, float %z) {
 ; FMF-LABEL: fmul_fadd_reassoc2:
 ; FMF:       # %bb.0:
-; FMF-NEXT:    xsmulsp 0, 1, 2
-; FMF-NEXT:    xsaddsp 1, 0, 3
+; FMF-NEXT:    xsmaddasp 3, 1, 2
+; FMF-NEXT:    fmr 1, 3
 ; FMF-NEXT:    blr
 ;
 ; GLOBAL-LABEL: fmul_fadd_reassoc2:
 ; GLOBAL:       # %bb.0:
 ; GLOBAL-NEXT:    xsmaddasp 3, 1, 2
 ; GLOBAL-NEXT:    fmr 1, 3
 ; GLOBAL-NEXT:    blr
   %mul = fmul reassoc float %x, %y

 ; fma(X, 7.0, X * 42.0) --> X * 49.0
 ; This is the minimum FMF needed for this transform - the FMA allows reassociation.
 
 ; FMFDEBUG-LABEL: Optimized lowered selection DAG: %bb.0 'fmul_fma_reassoc1:'
 ; FMFDEBUG:         fma {{t[0-9]+}}
 ; FMFDEBUG:       Type-legalized selection DAG: %bb.0 'fmul_fma_reassoc1:'
 
 ; GLOBALDEBUG-LABEL: Optimized lowered selection DAG: %bb.0 'fmul_fma_reassoc1:'
-; GLOBALDEBUG:         fmul unsafe {{t[0-9]+}}
+; GLOBALDEBUG:         fmul reassoc {{t[0-9]+}}
 ; GLOBALDEBUG:       Type-legalized selection DAG: %bb.0 'fmul_fma_reassoc1:'
 
 define float @fmul_fma_reassoc1(float %x) {
 ; FMF-LABEL: fmul_fma_reassoc1:
 ; FMF:       # %bb.0:
 ; FMF-NEXT:    addis 3, 2, .LCPI6_0@toc@ha
 ; FMF-NEXT:    addi 3, 3, .LCPI6_0@toc@l
 ; FMF-NEXT:    lfsx 0, 0, 3

 
 ; This shouldn't change anything - the intermediate fmul result is now also flagged.
 
 ; FMFDEBUG-LABEL: Optimized lowered selection DAG: %bb.0 'fmul_fma_reassoc2:'
 ; FMFDEBUG:         fma {{t[0-9]+}}
 ; FMFDEBUG:       Type-legalized selection DAG: %bb.0 'fmul_fma_reassoc2:'
 
 ; GLOBALDEBUG-LABEL: Optimized lowered selection DAG: %bb.0 'fmul_fma_reassoc2:'
-; GLOBALDEBUG:         fmul unsafe {{t[0-9]+}}
+; GLOBALDEBUG:         fmul reassoc {{t[0-9]+}}
 ; GLOBALDEBUG:       Type-legalized selection DAG: %bb.0 'fmul_fma_reassoc2:'
 
 define float @fmul_fma_reassoc2(float %x) {
 ; FMF-LABEL: fmul_fma_reassoc2:
 ; FMF:       # %bb.0:
 ; FMF-NEXT:    addis 3, 2, .LCPI7_0@toc@ha
 ; FMF-NEXT:    addi 3, 3, .LCPI7_0@toc@l
 ; FMF-NEXT:    lfsx 0, 0, 3

 
 ; The FMA is now fully 'fast'. This implies that reassociation is allowed.
 
 ; FMFDEBUG-LABEL: Optimized lowered selection DAG: %bb.0 'fmul_fma_fast1:'
 ; FMFDEBUG:         fma {{t[0-9]+}}
 ; FMFDEBUG:       Type-legalized selection DAG: %bb.0 'fmul_fma_fast1:'
 
 ; GLOBALDEBUG-LABEL: Optimized lowered selection DAG: %bb.0 'fmul_fma_fast1:'
-; GLOBALDEBUG:         fmul unsafe {{t[0-9]+}}
+; GLOBALDEBUG:         fmul reassoc {{t[0-9]+}}
 ; GLOBALDEBUG:       Type-legalized selection DAG: %bb.0 'fmul_fma_fast1:'
 
 define float @fmul_fma_fast1(float %x) {
 ; FMF-LABEL: fmul_fma_fast1:
 ; FMF:       # %bb.0:
 ; FMF-NEXT:    addis 3, 2, .LCPI8_0@toc@ha
 ; FMF-NEXT:    addi 3, 3, .LCPI8_0@toc@l
 ; FMF-NEXT:    lfsx 0, 0, 3

 
 ; This shouldn't change anything - the intermediate fmul result is now also flagged.
 
 ; FMFDEBUG-LABEL: Optimized lowered selection DAG: %bb.0 'fmul_fma_fast2:'
 ; FMFDEBUG:         fma {{t[0-9]+}}
 ; FMFDEBUG:       Type-legalized selection DAG: %bb.0 'fmul_fma_fast2:'
 
 ; GLOBALDEBUG-LABEL: Optimized lowered selection DAG: %bb.0 'fmul_fma_fast2:'
-; GLOBALDEBUG:         fmul unsafe {{t[0-9]+}}
+; GLOBALDEBUG:         fmul reassoc {{t[0-9]+}}
 ; GLOBALDEBUG:       Type-legalized selection DAG: %bb.0 'fmul_fma_fast2:'
 
 define float @fmul_fma_fast2(float %x) {
 ; FMF-LABEL: fmul_fma_fast2:
 ; FMF:       # %bb.0:
 ; FMF-NEXT:    addis 3, 2, .LCPI9_0@toc@ha
 ; FMF-NEXT:    addi 3, 3, .LCPI9_0@toc@l
 ; FMF-NEXT:    lfsx 0, 0, 3

 
 ; Reduced precision for sqrt is allowed - should use estimate and NR iterations.
 
 ; FMFDEBUG-LABEL: Optimized lowered selection DAG: %bb.0 'sqrt_afn:'
 ; FMFDEBUG:         fsqrt {{t[0-9]+}}
 ; FMFDEBUG:       Type-legalized selection DAG: %bb.0 'sqrt_afn:'
 
 ; GLOBALDEBUG-LABEL: Optimized lowered selection DAG: %bb.0 'sqrt_afn:'
-; GLOBALDEBUG:         fmul unsafe {{t[0-9]+}}
+; GLOBALDEBUG:         fmul reassoc {{t[0-9]+}}
 ; GLOBALDEBUG:       Type-legalized selection DAG: %bb.0 'sqrt_afn:'
 
 define float @sqrt_afn(float %x) {
 ; FMF-LABEL: sqrt_afn:
 ; FMF:       # %bb.0:
 ; FMF-NEXT:    xssqrtsp 1, 1
 ; FMF-NEXT:    blr
 ;

 
 ; The call is now fully 'fast'. This implies that approximation is allowed.
 
 ; FMFDEBUG-LABEL: Optimized lowered selection DAG: %bb.0 'sqrt_fast:'
 ; FMFDEBUG:         fsqrt {{t[0-9]+}}
 ; FMFDEBUG:       Type-legalized selection DAG: %bb.0 'sqrt_fast:'
 
 ; GLOBALDEBUG-LABEL: Optimized lowered selection DAG: %bb.0 'sqrt_fast:'
-; GLOBALDEBUG:         fmul unsafe {{t[0-9]+}}
+; GLOBALDEBUG:         fmul reassoc {{t[0-9]+}}
 ; GLOBALDEBUG:       Type-legalized selection DAG: %bb.0 'sqrt_fast:'
 
 define float @sqrt_fast(float %x) {
 ; FMF-LABEL: sqrt_fast:
 ; FMF:       # %bb.0:
 ; FMF-NEXT:    xssqrtsp 1, 1
 ; FMF-NEXT:    blr
 ;

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
 }