[PATCH] D82499: [DAGCombiner] tighten constraints for fma fold

[Sanjay Patel via Phabricator via llvm-commits]

spatel marked 2 inline comments as done.
spatel added inline comments.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp:11991
     E = N0;
   }
   if (FMA && E) {
----------------
cameron.mcinally wrote:
> Do we need to check `Aggressive` here? For a hypothetical target with 2 FMUL/FADD ports and 1 FMA port, assuming slow FMAs, this could be a performance loss.
> 
> It shouldn't be a problem for modern chips that I care about, so just picking nits.
Removing the 'Aggressive' clause was the previous patch. :) 
D80801

The reason for not requiring 'Aggressive' is that using FMA on this case is what we should assume is the best case for a default target that supports FMA. As discussed in the earlier patch, we know that this is difficult to get right for all code sequences/targets, so there is already an opt-out to bypass this in SDAG and use MachineCombiner instead. Potentially, we could also transform patterns like this after they have been fused to FMA. That would again be in MachineCombiner (where we have the detailed scheduler info).


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

https://reviews.llvm.org/D82499