[PATCH] D154805: [DAGCombiner] Fold IEEE `fmul`/`fdiv` by Pow2 to `add`/`sub` of exp

[Noah Goldstein via Phabricator via llvm-commits]

goldstein.w.n marked 4 inline comments as done.
goldstein.w.n added inline comments.


================
Comment at: llvm/include/llvm/CodeGen/TargetLowering.h:3971
+  //     -> (bitcast_to_FP (add (bitcast_to_INT C), Log2(Pow2) << mantissa))
+  // (fdiv C, (uitofp Pow2))
+  //     -> (bitcast_to_FP (sub (bitcast_to_INT C), Log2(Pow2) << mantissa))
----------------
arsenm wrote:
> I think this should just use the fmul case, and turn the compatible fdivs to fmul 
I'm not sure I understand. We will need an `fdiv` no matter what, at the very least for the reciprocal of the pow2.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp:16201
+
+  auto CastToVT = [&](EVT NewVT, SDValue ToCast) {
+    EVT CurVT = ToCast.getValueType();
----------------
arsenm wrote:
> I'd kind of expect there to be a helper that does this already?
There is `DAG.getZExtOrTrunc` but it doesn't check bitcast or if oldvt == newvt.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp:16488
 
+  if (SDValue R = combineFMulOrFDivWithIntPow2(this, this->TLI, N))
+    return R;
----------------
arsenm wrote:
> goldstein.w.n wrote:
> > arsenm wrote:
> > > Can you add a test where this w
> > Where this w?
> I moved the comment to the main box, the fma formation one
Okay, I added an fma test `fmul_pow_shl_cnt_vec_preserve_fma` and it does so (the call to this transform is explicitly after the fma fold. I added a comment as well to be clearer about that).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154805