[llvm] [X86][Codegen] - Introduce a new tuning for X86 backend to control FMA generation (PR #210343)

[Rohit Aggarwal via llvm-commits]

rohitaggarwal007 wrote:

> We already have enableAggressiveFMAFusion and isFMAFasterThanFMulAndFAdd tuning hooks, please don't try to add another.

Agreed that a new hook is the unwanted overhead, but I don't think either existing
hook can express what this patch is after, so I had like to check which direction
you'd prefer before I start a fix.

What I want is narrow: when contraction is enabled *only* globally
(-ffp-contract=fast), don't fuse; when the FMUL itself carries a contract flag
(pragma clang fp contract, -ffp-contract=on -> llvm.fmuladd), keep fusing. The
motivation is reduction chains like `acc += x * c[i]`: fusing pulls the multiply
onto the loop-carried dependency chain and raises its latency (FMA 4c vs FADD 3c),
whereas the separate FMUL issues in parallel off the chain. Explicit contraction
requests should still be honoured.

- enableAggressiveFMAFusion(VT) can only ever *widen* fusion. In
  visitFADDForFMACombine/visitFSUBForFMACombine it is used as
  `Aggressive || N->hasOneUse()` and to pick which of two FMULs to fold; every
  in-tree override returns true. There is no setting of it that suppresses a
  fusion that would otherwise happen.
- isFMAFasterThanFMulAndFAdd(MF, VT) is per function + type and never sees the
  node or its flags, so it is all-or-nothing. On X86 HasFMAD is always false
  (no FMAD), so returning false disables FMA formation outright, and
  SelectionDAGBuilder then also expands llvm.fmuladd (and the constrained/VP
  variants) to fmul+fadd. That throws away exactly the locally-requested
  contraction I am trying to preserve, so it is not a subset of the intended
  behaviour, it is a different one.

Options I see, happy to take whichever you prefer:
1. No new hook, but give the existing one the information it lacks: pass the
   SDNode (or the FMUL's SDNodeFlags) to isFMAFasterThanFMulAndFAdd so targets
   can answer per-node. That extends an existing hook instead of adding a knob,
   at the cost of touching all implementors.
2. Do the profitability analysis where the latency information lives: implement
   generateFMAsInMachineCombiner for X86 and form FMAs in the MachineCombiner as
   AArch64 does, so the critical path decides rather than a subtarget flag. This
   is the most principled fix and also the largest.
3. If you consider "global -ffp-contract=fast is a user request and a target
   should not second-guess it, but a target may say FMA is not faster", then
   option 3 is just returning false from X86's isFMAFasterThanFMulAndFAdd under
   the tuning feature, dropping the local-contract carve-out and updating the
   tests. I can do that if you are fine with pragma-level contract also being
   suppressed on these subtargets.

My preference is 1 in the short term and 2 as the real fix in long term.

Please let me know your views

https://github.com/llvm/llvm-project/pull/210343