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

[Sanjay Patel via Phabricator via llvm-commits]

spatel added a comment.

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

> @spatel  I remembered why I didn't add a transformation into DAGCombiner and decided to perform specific transformation in TargetLowering::SimplifySetCC:
>
> - to remove unnecessary shifts we have to know that legalized shift was an input of `setcc eq/ne 0` and expanded shifts are not yet combined when DAGCombiner is visiting setcc;
> - and it's actually better to apply the optimization before shifts combining because for some targets shifts could be combined to some target-specific node instead of a funnel shift (for example, for AArch64 shifts implemening funnel shift will be combined into EXTR node instead of FSHL/FSHR).
>
> As a generalization I was thinking to extract rotation-matching part of this optimization to a separate method that will actually combine shifts into rotations, call if from both TargetLowering::SimplifySetCC and  DAGCombiner::MatchRotate and then eliminate rotations in TargetLowering::SimplifySetCC.
> But it seems like or-shift expression trees that deep enough to apply such optimization should be quite rare. In fact, I'm hardly imagining cases other than `setcc eq/ne 0`.

I'm not convinced yet that we need to include the setcc to handle the motivating bug.

I noticed that we're missing an even more basic logic+shift transform (we miss this in IR too):
https://alive2.llvm.org/ce/z/RZjxTE

I think that transform should work with any shift opcode and any bitwise logic opcode (as long as they are matching opcodes within any one transform).

Would this patch still get the optimal result for the transformed code?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111530