[PATCH] D111530: [TargetLowering] Optimize expanded SRL/SHL fed into SETCC ne/eq 0

Fri Feb 18 10:15:32 PST 2022

fzhinkin added a comment.

In D111530#3332382 <https://reviews.llvm.org/D111530#3332382>, @fzhinkin wrote:

> In D111530#3329473 <https://reviews.llvm.org/D111530#3329473>, @spatel wrote:
>
>> I didn't step through everything here, so I may not be seeing the entire problem.
>>
>> There are 2 or more relatively simple folds that are missing both in IR and DAG, and adding those might solve this more generally and more easily than the proposed patch.
>>
>> Here are examples in Alive2:
>> https://alive2.llvm.org/ce/z/KNQuYm
>> https://alive2.llvm.org/ce/z/LKLpo3
>>
>> So it might be possible to solve the motivating bug without starting from icmp/setcc -- it's really a problem of combining shifts and funnel shifts in a way that is better for analysis/codegen.
>
> Transformation from this change is not only about shifts recombination but (and mostly) about removal of shifts that will rotate bits of a value compared with zero. In that particular case such shifts could be safely eliminated, but in any other case it's not possible as it will change the result.
>
> By combining shifts differently (by replacing shift + funnel shift with rotate + shift, for example) we can improve performance on some platforms, but the case with `icmp eq/ne 0` would still have to be handled separately as it allow transformation that is illegal otherwise.

On the other hand it's just a special case where we're using "identify" instead of rotation.

I agree that a general solution for shifts combining could be developed and used instead of the transformation from this change.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111530