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

[Noah Goldstein via Phabricator via llvm-commits]

goldstein.w.n added a comment.

In D154805#4612046 <https://reviews.llvm.org/D154805#4612046>, @arsenm wrote:

> In D154805#4611744 <https://reviews.llvm.org/D154805#4611744>, @goldstein.w.n wrote:
>
>> why is that? ldexp gets lowered as mul/div and its not really possible to transform ldexp into this.
>
> It doesn't lower to mul/div (and it may need a range check, at least for div). Most targets use the libcall

Err was thinking implemented with, but thats wrong too.
Looks like ldexp is implemented with the same trick as here (at least glibc/llvm-libc).
We are essentially inlining the fast path.
So don't really see how this could cause a slowdown given it gets to skip all the
checks + is inlined.
I can see the argument against having it in instcombine, but as its implemented here,
its at the end of the line so the speak and there isn't much left for it to potentially
de-optimize.

> It's approximately
>
>   fmul x, pow2_k => ldexp(x, log2(pow2_k)
>   fmul x, pow2_k => ldexp(x, -log2(pow2_k)
>
> with a possible additional precondition for the exponent range

I tried implemented this through `ldexp` but ALOT of powers of two are known from shifts
and its quite useful to do the analysis when its still in shift state as we can easily limit
the exponent.

If there where flags attached to ldexp that bounded the exponent I agree it would be
best to get to `ldexp` in InstCombine then use `ldexp` for the lowering.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154805