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

[Sergei Barannikov via Phabricator via llvm-commits]

barannikov88 added inline comments.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp:7856
+      unsigned int Rshr = *Lshr.getRawData() >> *ShiftAmt.getRawData();
+      RshrArr[Rshr] = i;
+    }
----------------
gsocshubham wrote:
> Regarding this line - `RshrArr[Rshr] = i;`
> 
> In debug mode, when I run `SPARC/cttz.ll` which is present in this patch, I get a segmentation fault. but in release it is working fine. 
> 
> Why is there a difference in the behaviour?
Most probably there is a bug in your code. Most probably you haven't enabled assertions in release mode (-DLLVM_ENABLE_ASSERTIONS=ON is the default in debug mode).
You should be able to find some hints in the printed backtrace, but first make sure that `llvm-symbolizer` target is built.



================
Comment at: llvm/test/CodeGen/VE/Scalar/cttz.ll:44
 ; CHECK-NEXT:    and %s0, %s0, (32)0
-; CHECK-NEXT:    pcnt %s0, %s0
 ; CHECK-NEXT:    b.l.t (, %s10)
----------------
gsocshubham wrote:
> dmgreen wrote:
> > If the target has a ctpop instruction then that should be preferred to the table lookup.
> I have added a check - `!isOperationLegal(ISD::CTPOP, VT)` but that does not seem to help.
VE sets the action to `Promote`. Perhaps, you should call `isOperationExpand` instead and do the transformation if it returns true.


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

https://reviews.llvm.org/D128911