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

Michael Kitzan via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Sep 17 16:47:16 PDT 2020


mkitzan 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);
+
----------------
arsenm wrote:
> 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
I see. I was wondering about the discrepancy between the `G_MUL` version and this one, but chalked it up floating point stuff. I'll post a fixup changing it to fold into `G_FSUB(0.0, x)`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87870



More information about the llvm-commits mailing list