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

[Dave Green via Phabricator via llvm-commits]

dmgreen added a comment.

Can you add some tests?

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.



================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:17460
+  SDValue N2 = N->getOperand(2);
+  bool isZero = cast<ConstantSDNode>(N2.getNode())->isZero();
+  if (N1.getOpcode() == ISD::CTTZ && isZero) {
----------------
As far as I can tell this is checking that the condition code is 0? It would be better to check that it is equal to AArch64CC::EQ.


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:17462
+  if (N1.getOpcode() == ISD::CTTZ && isZero) {
+    SDValue thirtyOne = DAG.getConstant(31, SDLoc(N), N1.getValueType());
+    return DAG.getNode(ISD::AND, SDLoc(N), N1.getValueType(), N1, thirtyOne);
----------------
Variables in llvm start with capital letters. We should make sure that i64 work too, it needs a different constant (there should be tests too).


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

https://reviews.llvm.org/D123782