[PATCH] D128911: Emit table lookup from TargetLowering::expandCTTZ()

[Sergei Barannikov via Phabricator via llvm-commits]

barannikov88 added inline comments.


================
Comment at: llvm/include/llvm/CodeGen/TargetLowering.h:4725
+  /// \param N Node to expand
+  /// \returns Reference to table generated in Constant Pool.
+  SDValue CTTZTableLookup(SDNode *N, SelectionDAG &DAG, SDLoc dl, EVT VT,
----------------
I believe it returns something else


================
Comment at: llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp:7796
+  APInt BitWidth(32, NumBitsPerElt);
+  APInt ShiftAmt(32, (BitWidth - BitWidth.logBase2()).getZExtValue());
+  SDValue Neg = DAG.getNode(ISD::SUB, dl, VT, DAG.getConstant(0, dl, VT), Op);
----------------
You should be able to avoid getZExtValue here either.
Moreover, you don't need APInt here at all, the values are small enough to be put into 'unsigned', e.g.:
`unsinged ShiftAmt = BitWidth - Log2_32(BitWidth);`
Same for most other APInts.



================
Comment at: llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp:7805
+  // Create a table in constant data pool
+  std::vector<Constant *> Elts;
+  uint8_t RshrArr[32];
----------------
Could as well be C array.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp:7809
+  for (unsigned int i = 0; i < NumBitsPerElt; i++) {
+    APInt Lshr = DeBruijn.rotl(i);
+    unsigned int Rshr = Lshr.getZExtValue() >> ShiftAmt.getZExtValue();
----------------
There is no standalone "rotl" function which would work for 'unsigned', but you could do something like this:
`Lshr = (DeBruijn << i) | (DeBruijn >> (NumBitsPerElt - Amt)));



================
Comment at: llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp:7839
   unsigned NumBitsPerElt = VT.getScalarSizeInBits();
+  const TargetLowering &TLI = DAG.getTargetLoweringInfo();
 
----------------
This is redundant. Your code is already in TargetLowering class. Just call `isOperationExpand` directly.


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

https://reviews.llvm.org/D128911