[PATCH] D71495: [SelectionDAG] Copy FP flags when visiting a binary instruction.

[Sanjay Patel via Phabricator via llvm-commits]

spatel added a comment.

In D71495#1790020 <https://reviews.llvm.org/D71495#1790020>, @vchuravy wrote:

> In D71495#1788333 <https://reviews.llvm.org/D71495#1788333>, @spatel wrote:
>
> > It's not clear to me where the bug is. I see this debug output:
> >
> >   Detected a reduction operation:  %34 = fadd fast <4 x double> %vec.phi, %30
> >   Creating new node: t36: v4f64 = fadd vector-reduction t35, t30 <-- dropped all other FMF
> >   
> >
> > So that seems like we just accidentally cleared FMF by setting vector-reduction. If so, then a better fix would be after line 3127?
>
>
> We dropped all FMF because we created a new `SDNodeFlags Flags` and then create a new node with those flags and replace the input node with the new one.
>  Setting the vector reduction doesn't clear the FMF, so where we copy them doesn't matter much. I can of course move the copying of the FMF after setting the vector reduction flag.


Ok, I see how this fails now - back in the parent SelectionDAGBuilder::visit() function, we intersect flags for nodes that already have flags, so we are intersecting 'fast' with 'vector-reduction' which is nothing.

> I reduced the test as much as possible using `llvm-reduce`, I had to check whether a patched binary produced and fma and a unpatched binary not yet produced a fma.
>  If the test is missing the middle block the fma is generated in either case.

We have to trigger isVectorReductionOp() to be true, but that doesn't require separate basic blocks. I added a further reduced test here:
rG13d30bd54b8b <https://reviews.llvm.org/rG13d30bd54b8b4903255fdb6e09d9719aeceda4a3>

That covers the only code path for this bug, so you should be able to rebase and update that test file by running utils/update_llc_test_checks.py.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71495