[llvm] [InstCombine] Canonicalize reassoc contract fmuladd to fmul + fadd (PR #90434)

Andy Kaylor via llvm-commits llvm-commits at lists.llvm.org
Mon May 6 10:00:19 PDT 2024


andykaylor wrote:

> so I think that is what  @andykaylor expected, ie:  clang should not generate the llvm.fmuladd intrinsic in the fp-contract=fast state, and **fast @llvm.fmuladd.v2f64** in IR is also not expected? not only the **contract @llvm.fmuladd.v2f64** ?

Perhaps what I said was unclear. It is perfectly legal in the IR to have the `fast` flag or the `contract` flag attached to the llvm.fmuladd intrinsic, and the canonicalization you are proposing here is good in those case. In fact, I don't think it's necessary to even check for the presence of the `reassoc` flag. What I was saying is that, as far as I know, clang **does not** generate llvm.fmuladd() in the fp-contract=fast state, and so if we are seeing that in the IR in a case where clang was used as the front-end, it probably indicates a bug somewhere in an intermediate optimization.

If some other front end is generating llvm.fmuladd() with the `contract` flag set, I'd want to understand what they intended to accomplish by generating such IR. It seems like they may have actually intended llvm.fma() which should not be split.

The canonicalization is good, but I would prefer to track down the source of the unexpected IR before the change is made to hide a potential problem. It might even be good to emit an optimization remark saying we've done this, because I don't think there is any good reason for this IR to have been used.

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


More information about the llvm-commits mailing list