[PATCH] D64275: [InstCombine] Generalize InstCombiner::foldAndOrOfICmpsOfAndWithPow2().

[Roman Lebedev via Phabricator via llvm-commits]

lebedev.ri marked an inline comment as done.
lebedev.ri added inline comments.


================
Comment at: llvm/test/Transforms/InstCombine/onehot_merge.ll:246
 
 ; Should not fold
 define i1 @foo1_and_extra_use_and(i32 %k, i32 %c1, i32 %c2, i32* %p) {
----------------
huihuiz wrote:
> lebedev.ri wrote:
> > Looks like this one should?
> I am OK with allowing 'and' to have more than one use.
> 
> Folding cut instruction count by 2, the worst case is no instruction count increase.
> the worst case is no instruction count increase.

Precisely.


================
Comment at: llvm/test/Transforms/InstCombine/onehot_merge.ll:340
 
 ; Should not fold
 define i1 @foo1_and_extra_use_cmp2(i32 %k, i32 %c1, i32 %c2, i1* %p) {
----------------
huihuiz wrote:
> lebedev.ri wrote:
> > Could fold, no instruction count increase
> We should not fold when 'cmp' has more than one use, this will increase instruction count when one side is using the new form.
> 
> Take this as an example, we need to keep the extra 'cmp' that can't be reused, while the benefit of folding is offset by the added signbit shift instruction.
> 
> ```
> define i1 @foo(i32 %k, i32 %c1, i32 %c2, i1* %p) {
>   %t0 = shl i32 1, %c1
>   %t1 = and i32 %t0, %k
>   %t2 = icmp eq i32 %t1, 0
>   store i1 %t2, i1* %p  ; extra use of cmp
>   %t3 = shl i32 %k, %c2
>   %t4 = icmp sgt i32 %t3, -1
>   %or = or i1 %t2, %t4
>   ret i1 %or
> }
> 
> ```
I believe i'm missing the point in `@foo1_and_extra_use_cmp2`.
We were doing this fold previously (in llvm trunk), now we don't because of use-count.
But this restriction does not seem to improve anything?
We had 8 instructions, and we have 8 instructions without the fold?



Repository:
  rL LLVM

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

https://reviews.llvm.org/D64275