[PATCH] D108284: [DAGCombiner] Combine frem into fdiv+ftrunc+fma

[Qiu Chaofan via Phabricator via llvm-commits]

qiucf added inline comments.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp:14425
 
+  // (frem x, y) -> (fma (fneg (ftrunc (fdiv x, y))), y, x)
+  if (Flags.hasApproximateFuncs() &&
----------------
foad wrote:
> foad wrote:
> > Is this always preferable, even on targets where FREM is legal?
> I see you have added an isLegal check for FREM. But I don't understand why you are doing this as a combine in the first place, instead of changing the way FREM is legalized to do this as a lowering instead of calling a libcall.
I'm not sure it's good idea to consider fast-math flags in legalizing.. Here `frem` just looks like how `fdiv` is transformed into series of operations in combiner.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108284