[PATCH] D87870: [GISel] Add new combines for G_FMUL

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added inline comments.


================
Comment at: llvm/include/llvm/CodeGen/GlobalISel/CombinerHelper.h:358-359
 
+  /// Transform G_FMUL(x, -1) to G_FNEG(x)
+  bool applyCombineFMulByNegativeOne(MachineInstr &MI);
+
----------------
mkitzan wrote:
> arsenm wrote:
> > This is incorrect since it replaces a canonicalizing operation with a non-canonicalizing operation
> Worth noting there is an [identical DAGCombiner for this](https://github.com/llvm/llvm-project/commit/e49c974a7c8705cdd0db6202fc891aaa56401d89#diff-de23abfb0f87df1b6be71b792800344bR2903) though it was committed 13yrs ago. Is this a GISel specific requirement (not replacing a canonicalizing op with a non-canonicalizing op)? Regardless, it can easily be turned into `G_FMUL(x, -1.0)` -> `G_FSUB(0.0, x)`.
Until recently, fneg and fsub -0, x were considered identical and didn't try to get this right. The DAG is broken here


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87870