[PATCH] D124183: [InstCombine] Add one use limitation for (X * C2) << C1 --> X * (C2 << C1)

chenglin.bi via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Apr 21 12:04:44 PDT 2022


bcl5980 added a comment.

In D124183#3465319 <https://reviews.llvm.org/D124183#3465319>, @lebedev.ri wrote:

> In D124183#3465290 <https://reviews.llvm.org/D124183#3465290>, @bcl5980 wrote:
>
>> One other reason I insist one-use is we can give programmer choice. If we remove one-use, there is no choice for programmer.
>> https://godbolt.org/z/Wrzh5aczf
>> https://godbolt.org/z/1nGf88E87
>
> Apologies, i do not really understand what you mean by a choice here.

Programmer can determine the final result is mul+shift or two independent mul.
if they write with a chain we compile to mul+shift chain, and if they write 2 independent mul we compile to 2 mul

  unsigned a = i * 100; -->    mul 
  unsigned b = a * 4;   -->    shift



  unsigned a = i * 100;   -->    mul1
  unsigned b = i * 400;   -->    mul2

Let programmer to decide which pattern is better. I'm pretty sure on GPU(CUDA/HIP) or power sensitive CPU plaform we prefer mul+shift.
Otherwise they need backend to do this for all these platform.

> In D124183#3465313 <https://reviews.llvm.org/D124183#3465313>, @bcl5980 wrote:
>
>> @spatel Do you agree with @lebedev.ri to revert both one-use? If yes I will revert both of them.
>
> Note that i was not asking for the revert, and there is really no need to revert the D123453 <https://reviews.llvm.org/D123453>, if needed it can be adjusted in a new commit.

Yeah, I mean do we need a new commit to remove one-use on D123453 <https://reviews.llvm.org/D123453>.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124183



More information about the llvm-commits mailing list