[PATCH] D87200: [SelectionDAGBuilder] Pass fast math flags to getNode calls rather than trying to set them after the fact.

[Sanjay Patel via Phabricator via llvm-commits]

spatel accepted this revision.
spatel added a comment.
This revision is now accepted and ready to land.

LGTM, but please wait at least a day to commit in case there are other comments.



================
Comment at: llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp:5010
       // error 0.0049451742, which is more than 7 bits
       SDValue t0 = DAG.getNode(ISD::FMUL, dl, MVT::f32, X,
                                getF32Constant(DAG, 0xbeb08fe0, dl));
----------------
craig.topper wrote:
> spatel wrote:
> > Should use the incoming Flags on all created nodes now.
> Can we do that in a follow up? There's a quite a bit of code in these approximations. Adding the flags could enable FMA formation or reassociation that we weren't doing before.
Yes - but on 2nd thought, we probably shouldn't do that on any of these expansions. IIRC, we had to undo the FMF propagation on some of the x86 cast expansions to avoid bungling the expected bit-hacks. We should just assume these expansions are the best as-is.


================
Comment at: llvm/test/CodeGen/PowerPC/fmf-propagation.ll:560
 ; FMFDEBUG:         ch,glue = callseq_end t15, TargetConstant:i64<32>, TargetConstant:i64<0>, t15:1
-; FMFDEBUG:         f64,ch,glue = CopyFromReg afn t16, Register:f64 $f1, t16:1
+; FMFDEBUG:         f64,ch,glue = CopyFromReg t16, Register:f64 $f1, t16:1
 ; FMFDEBUG:       Type-legalized selection DAG: %bb.0 'log2_approx:'
----------------
craig.topper wrote:
> spatel wrote:
> > We need to find where this flag got dropped?
> It was getting a FMF flag before because it was the last instruction created by handling a call. So the after the fact handling found it. I can’t think of a reason we need FMF on a copy. If we need them anywhere it would be on the CALL_NOP right?
Yes, that makes sense. If we need a flag, it should be on the call, so that's independent of this patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87200