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

[Craig Topper via Phabricator via llvm-commits]

craig.topper added inline comments.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp:8025
+  // Emit Table Lookup if ISD::CTLZ and ISD::CTPOP are not legal.
+  if (NumBitsPerElt == 32 && !VT.isVector() &&
+      TLI.isOperationExpand(ISD::CTPOP, VT) && !isOperationLegal(ISD::CTLZ, VT))
----------------
gsocshubham wrote:
> craig.topper wrote:
> > gsocshubham wrote:
> > > craig.topper wrote:
> > > > Should this be checking CTTZ_ZERO_UNDEF? The zero case is not handled correctly by the table lookup. For CTTZ we need a select. CodeGenPrepare rewrites llvm.cttz(i32 %x, i1 false) into a branch around llvm.cttz(i32 %x, i1 true) on some targets. So the difference might be hard to test.
> > > Can you point me to target and test where above scenario occurs? I will update it accordingly.
> > Compiling llvm.cttz.i32 for riscv32 with -O0 should do it I think. -O0 will disable codegen prepare.
> @craig.topper - What is the desired output value from the table when x=0?
> 
> ```
> define i32 @f(i32 %x) {
> entry:
>   %0 = call i32 @llvm.cttz.i32(i32 %x, i1 true)
>   ret i32 %0
> }
> ```
With `i1 true` there is no desired out put for 0. I'm concerned about `i1 false` which needs to produce 32.



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

https://reviews.llvm.org/D128911