[PATCH] D80801: [DAGCombiner] allow more folding of fadd + fmul into fma

[Sanjay Patel via Phabricator via llvm-commits]

spatel added a reviewer: PowerPC.
spatel added a comment.

In D80801#2102362 <https://reviews.llvm.org/D80801#2102362>, @bryanpkc wrote:

> In D80801#2102291 <https://reviews.llvm.org/D80801#2102291>, @efriedma wrote:
>
> > > We are only pulling the trailing addition in with an existing multiply.
> >
> > The problem here is that it's the "wrong" multiply: you have, essentially `(A*B+D*E)+F`, and you're turning it into `A*B+(D*E+F)`.  I don't think contraction is supposed to cover that.
>
>
> That is exactly my concern; I was under the impression that contraction only allowed fusing `(A*B+C)` and nothing else.


Here's a playground to check various compilers/orderings:
https://godbolt.org/z/3nV8Mx

So I was wrong - gcc trunk is not forming 2 fmas on "ab + cd + e". Neither is icc (if I specified the optimization options correctly).
AFAIK, there's no formal or even loose specification for "-ffp-contract=fast", so we probably want to follow gcc's lead here? Ie, this transform should require "reassoc"; "contract" is not enough.

Note that this patch did not actually change the transform behavior - it only enabled the existing transform on more targets. So if we require looser FP settings to do the transform, it may regress targets like PowerPC.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80801