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

[David Majnemer via llvm-commits]

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.


Repository:
  rL LLVM

http://reviews.llvm.org/D15718