[PATCH] D123782: [AArch64] Generate AND in place of CSEL for Table Based CTTZ lowering in -O3

Sat Apr 23 15:31:14 PDT 2022

craig.topper added a comment.

In D123782#3468601 <https://reviews.llvm.org/D123782#3468601>, @rahular-rrlogic wrote:

> In D123782#3467076 <https://reviews.llvm.org/D123782#3467076>, @dmgreen wrote:
>
>>>> As far as I understand this should be testing for select (icmp eq X, 0), 0, cttz X and converting it to and(cttz X, #bw-1). If so there are more conditions that need to be checked.
>>>
>>> What other conditions do you think should be added other than checking for 0 and cttz?
>>
>> It looks like this checks for the select/CSEL, and the icmp/SUBS with a 0, but not the "eq" yet, and not the "0" in the select/csel. It also doesn't check that the X in the icmp and the X in the cttz are the same?
>
> Is the last check really needed? Both icmp and cttz use the value from the same register in the IR itself.

Yes it's needed. You have to make you don't optimize `select (icmp eq X, 0), 0, cttz Y` if X and Y aren't the same.

================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:17469
+         N1.getOperand(0).getOpcode() == ISD::CTTZ)) {
+      unsigned BitWidth =
+          cast<ConstantSDNode>(N0)->getConstantIntValue()->getBitWidth();
----------------
You can get the bitwidth a lot quicker with N1.getValueSizeInBits()

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

https://reviews.llvm.org/D123782