[PATCH] D103788: [InstCombine] Eliminate casts to optimize ctlz operation

[Datta Nagraj via Phabricator via llvm-commits]

datta.nagraj marked 4 inline comments as done.
datta.nagraj added a comment.

Added more tests as per review comments.



================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineCasts.cpp:954
+  // trunc (ctlz_i32(zext(A), B) --> add(ctlz_i16(A, B), C)
+  if (match(Src, m_OneUse(m_Intrinsic<Intrinsic::ctlz>(m_ZExt(m_Value(A)),
+                                                       m_Value(B))))) {
----------------
RKSimon wrote:
> We will need to add multiuse tests as well to check that that the m_OneUse is working correctly
Done. Added 3 tests at the end which have multiple uses.


================
Comment at: llvm/test/Transforms/InstCombine/zext-ctlz-trunc-to-ctlz-add.ll:71
+  ret i16 %zz
+}
----------------
RKSimon wrote:
> We need vector type coverage as well - at least fixed vectors, scalable vectors as well would be a bonus
> 
> declare <2 x i32> @llvm.ctlz.v2i32 (<2 x i32>, i1)
> 
> declare <vscale x 2 x i32> @llvm.ctlz.v2i32 (<vscale x 2 x i32>, i1)
Done. Added tests with vector and scalable vectors.


================
Comment at: llvm/test/Transforms/InstCombine/zext-ctlz-trunc-to-ctlz-add.ll:71
+  ret i16 %zz
+}
----------------
datta.nagraj wrote:
> RKSimon wrote:
> > We need vector type coverage as well - at least fixed vectors, scalable vectors as well would be a bonus
> > 
> > declare <2 x i32> @llvm.ctlz.v2i32 (<2 x i32>, i1)
> > 
> > declare <vscale x 2 x i32> @llvm.ctlz.v2i32 (<vscale x 2 x i32>, i1)
> Done. Added tests with vector and scalable vectors.
Done. Added tests with vector and scalable vectors.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103788