[PATCH] D15718: Recognize patterns for ctpop and ctlz in instcombine

David Majnemer via llvm-commits llvm-commits at lists.llvm.org
Tue Dec 22 10:25:38 PST 2015

majnemer added a comment.

Unless I am mistaken, the ctpop and ctlz pieces of this code are independent of one another.  Please split the patch into two patches, one for each transform.

Comment at: lib/Transforms/InstCombine/InstCombineAddSub.cpp:1181-1183
@@ -1053,2 +1180,5 @@
+  if (Value *V = SimplifyToCtpop(&I, Builder))
+    return ReplaceInstUsesWith(I, V);
   if (Value *V = SimplifyVectorOp(I))
Seeing as how this is a rather rare occurrence for adds, can you sort this to the bottom?  The `Simplify...` naming convention implies that no new instruction needs to be created which is not the case for this transform.  I'd call it `optimizeCtpopIdiom` or something similar.

Comment at: test/Transforms/InstCombine/ctpop-match.ll:56
@@ +55,3 @@
+; CHECK: define i32 @pop16
+; CHECK: ctpop
+define i32 @pop16(i16 zeroext %t0) #0 {
Please CHECK a little more thoroughly, it would be nice to see the call to ctpop consume the argument, etc.

Comment at: test/Transforms/InstCombine/ctpop-match.ll:94-96
@@ +93,5 @@
+  %add14 = add i32 %and11, %and13
+  store i32 %add14, i32* %t4, align 4
+  %8 = load i32, i32* %t4, align 4
+  ret i32 %8
This test case doesn't look reduced.



More information about the llvm-commits mailing list