[PATCH] D159283: Add intrinsic to count trailing zero elements in a vector

[Nikita Popov via Phabricator via llvm-commits]

nikic added inline comments.


================
Comment at: llvm/include/llvm/IR/Intrinsics.td:2176
+  DefaultAttrsIntrinsic<[llvm_anyint_ty],
+            [llvm_anyvector_ty, llvm_i32_ty],
+            [IntrNoMem, IntrNoSync, IntrWillReturn, ImmArg<ArgIndex<1>>]>;
----------------
kmclaughlin wrote:
> craig.topper wrote:
> > why i32 instead of i1 like the existing cttz intrinsic?
> Hi @craig.topper, I tried adding this argument as an i1, but ran into problems when trying to build the patterns in AArch64SVEInstrInfo.td if I tried to match this type. I tried to find some similar intrinsics but didn't find any with patterns that match an i1, which is why I chose to use an i32 with `timm32_0_1`.
Can't comment on how to do this in tablegen, but as this is a target-independent intrinsic, we'd definitely want the argument to be i1 and not i32.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp:7548
+          DAG.getSelectCC(DL, Sub, VL, DAG.getUNDEF(NewEltTy), Sub, ISD::SETEQ);
+    }
+
----------------
I don't think this bit makes sense. The zero-is-poison flag *allows* you to return poison if this input is zero, but you don't have to return it. Setting this kind of flag should never result in more complex code, as you are generating here.


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:5343
+    assert(cast<ConstantSDNode>(getValue(I.getOperand(1)))->getZExtValue() !=
+           0);
+
----------------
I don't get what guarantees this assertion (or why it would make sense to assert this at all).


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

https://reviews.llvm.org/D159283