[PATCH] D37686: [DAG] Consolidating Instruction->SDNode Flags propagation in one class for better code management.

[Jatin Bhateja via Phabricator via llvm-commits]

jbhateja added a comment.

In https://reviews.llvm.org/D37686#877864, @spatel wrote:

> I've added some more FMF tests at https://reviews.llvm.org/rL313893 which I think this patch will miscompile. Please rebase/update.
>
> As I suggested before, this patch shouldn't try to enable multiple DAG combines with node-level FMF. It's not as straightforward as you might think.
>
> Pick exactly one combine if you want to show that this patch is working as intended. The llvm.muladd intrinsic test that you have here with a target that supports 'fma' (not plain x86) seems like a good choice to me. If we have a strict op in IR, it should produce an fma instruction. If we have a fast op in IR, it should produce the simpler fmul instruction?


My understanding and code changes are based LLVM Ref Manual 's section about Fast-Math flags"  (http://llvm.org/docs/LangRef.html#fast-math-flags)

Which say for FMF flag NaN  "Allow optimizations to assume the arguments and result are not NaN".

Now in following case which has been added by you

  %y = call float @llvm.sqrt.f32(float %x)
  %z = fdiv fast float 1.0, %y
  ret float %z

We dont have fast flag over intrinsic but DAGCombining for fdiv sees a fast flag and assume result (%z) and arguments (constant , %y) as not a Nan and goes ahead and generates a reciprocal sqrt. If you remove fast from fdiv and add it to intrinsic then FMF opt at fdiv will not kick in.

Can you please let me know what you expected here.


https://reviews.llvm.org/D37686