[PATCH] D130232: Reassoc FMF should not optimize FMA(a, 0, b) to (b)

[Sven van Haastregt via Phabricator via llvm-commits]

svenvh added inline comments.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp:15036
 
-  if (UnsafeFPMath) {
+  if (Options.UnsafeFPMath) {
     if (N0CFP && N0CFP->isZero())
----------------
spatel wrote:
> We are slowly trying to get away from using the global settings by replacing those checks with fast-math-flags. Can we do that here?
> 
> This should match the conditions that we use to simplify X * 0.0. That's done via DAG.simplifyFPBinop():
> 
>   // X * 0.0 --> 0.0
>   if (Opcode == ISD::FMUL && Flags.hasNoNaNs() && Flags.hasNoSignedZeros())
>     if (YC->getValueAPF().isZero())
>       return getConstantFP(0.0, SDLoc(Y), Y.getValueType());
> 
Sure, but is it okay to do the modernization in a separate followup commit?  This change just intends to fix the immediate bug (i.e. restore the status quo to before 8e570c3390d20, which is where this bug was introduced).

I guess the followup patch will be like this then?
```
@@ -15026,7 +15026,7 @@ SDValue DAGCombiner::visitFMA(SDNode *N) {
        CostN1 == TargetLowering::NegatibleCost::Cheaper))
     return DAG.getNode(ISD::FMA, DL, VT, NegN0, NegN1, N2);
 
-  if (Options.UnsafeFPMath) {
+  if (N->getFlags().hasNoNaNs() && N->getFlags().hasNoSignedZeros()) {
     if (N0CFP && N0CFP->isZero())
       return N2;
     if (N1CFP && N1CFP->isZero())
```


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D130232/new/

https://reviews.llvm.org/D130232