[PATCH] D127801: [InstCombine] convert mask and shift of power-of-2 to cmp+select

[Sanjay Patel via Phabricator via llvm-commits]

spatel added a comment.

In D127801#3585983 <https://reviews.llvm.org/D127801#3585983>, @bcl5980 wrote:

> Actually even if the instruction number are the same, we still prefer shift+and because cmp port is less than shift for most modern CPU I think.

Yes, some targets may have better throughput, but I don't think there is enough difference to change the decision in IR.

As a counter-example, consider the horrible result for the shift+and pattern with x86 vectors (a target that may not have good support for variable shifts):
https://alive2.llvm.org/ce/z/ciMo-S

So we just have to decide if this canonicalization is worthwhile to improve IR. The backend has to do some work either way to produce optimal code for all targets.

> And I try to create 2 cases: https://alive2.llvm.org/ce/z/exvD7T

Thanks - I understand your point. Although for RISCV64, I don't think we should see the sign extensions in a normal example from source:
https://godbolt.org/z/KWEPPTsx1

> Maybe we can add metadata MD_range for x to keep the information then backend can get enough information to invert the transform.

I haven't looked at metadata in a long time, so I don't know if it is practical. Range metadata is limited on where it can be added (for example, not on function parameters?), and then we have to make sure it is still available for the backend to use.

Since we made similar transforms before this one, this seemed like a good canonicalization to me, but if you think there's too much risk for the backend, we can abandon. If we do that, then we'd likely want to recreate this transform in SDAG, so we can still improve examples like the x86 code I showed in the previous link.


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

https://reviews.llvm.org/D127801