[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 13 inline comments as done.
goldstein.w.n added inline comments.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp:16158
+      switch (V.getOpcode()) {
+      case ISD::BITCAST:
+      case ISD::TRUNCATE:
----------------
RKSimon wrote:
> If we allow bitcast can't we end with cases where we've flipped between vectors/scalars or changed vectorelementcount?
Dropped bitcast peeking here. It doesn't really add any value b.c we can't handle any of the cases that bitcast would be needed (Int <-> Vec <-> FP).


================
Comment at: llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp:16256
+    ConstOp = peekThroughBitcasts(N->getOperand(ConstOpIdx));
+    Pow2Op = peekThroughBitcasts(N->getOperand(1 - ConstOpIdx));
+    if (Pow2Op.getOpcode() != ISD::UINT_TO_FP &&
----------------
RKSimon wrote:
> Don't you need to ensure that the scalar size in bits of stays the same through the bitcast? AFAICT you should only accept bitcasts that casts from fp to/from int (both scalar/vector and same scalar width)?
When we takelog2 it will cast the new SDValue to proper integer type. But dropping peekthroughbitcasts here as it doesn't really add any value.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154805