[PATCH] D46760: [InstCombine] Enhance narrowUDivURem.

[Roman Lebedev via Phabricator via llvm-commits]

lebedev.ri added a comment.

In https://reviews.llvm.org/D46760#1105919, @spatel wrote:

> You want to narrow multi-use sequences of code because instcombine can't do that profitably using minimal peepholes:
>
>   Name: narrow expression
>   Pre: C1 u< 9945
>     %tmp2 = zext i32 C1 to i64
>     %lane_id = and i64 %tmp2, 31
>     %shr = lshr i64 %tmp2, 2
>     %tmp4 = shl nuw nsw i64 %shr, 5
>     %r = or i64 %lane_id, %tmp4
>   =>
>     %nlane_id = and i32 C1, 31
>     %nshr = lshr i32 C1, 2
>     %ntmp4 = shl i32 %nshr, 5
>     %ntmp5 = or i32 %nlane_id, %ntmp4
>     %r = zext i32 %ntmp5 to i64
>
>
> The original motivation for -aggressive-instcombine (TruncInstCombine) was something almost like that - see https://reviews.llvm.org/D38313. Can you extend that? Note that the general problem isn't about udiv/urem, lshr, or any particular binop. It's about narrowing a sequence of arbitrary binops (and maybe even more than binops).


Hmm, there already is `llvm/lib/Transforms/AggressiveInstCombine/TruncInstCombine.cpp`.
I'm guessing what you meant is, can that be extended to handle this case, too?
It currently does not https://godbolt.org/g/CMd7sK

@bixia did you look into that yet by any chance?

> Re: instcombine - if some transform is increasing instruction count by not checking for uses, that's a bug. I think we established that conclusively in https://reviews.llvm.org/D44266.
> 
> The problem with trying to do this transform in instcombine by checking the users is that if it's ok to check the users for this 1 transform, then why don't we do that for *every* transform? The compile-time cost of that change would be substantial, and we're trying to limit the cost of instcombine. See recent llvm-dev discussions for more details.




Repository:
  rL LLVM

https://reviews.llvm.org/D46760