[PATCH] [AArch64] Implement popcount pattern

[Kyrill Tkachov]

Hi Naveen,

On 12/12/16 03:16, Hurugalawadi, Naveen wrote:
> Hi,
>
> Please find attached the patch that implements the support for popcount
> patterns in AArch64.
>
> The implementation improves OVS-DPDK on ThunderX by 3%. It would have a
> similar effect on other AArch64 targets.
>
> Please review the patch and let us know if its okay?

Besides a few comments below this looks good to me if it has gone through
the normal required bootstrap and testing, but I can't approve.
It's at the discretion of the target maintainers/reviewers if they want to
accept it at this stage.

> 2016-12-12  Andrew Pinski  <apinski@cavium.com>
>
> gcc
> 	* config/aarch64/aarch64.md (popcount<mode>2): New pattern.
>
> gcc/testsuite
> 	* gcc.target/aarch64/popcount.c : New Testcase.

diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md
index 65eb326..c688ddc 100644
--- a/gcc/config/aarch64/aarch64.md
+++ b/gcc/config/aarch64/aarch64.md
@@ -3785,6 +3785,39 @@
    }
  )
  
+/* Pop count be done via the pop count instruction in NEON. */

The rest of the MD file uses the term AdvSIMD. Also, the instrurction
is CNT rather than "pop count".

+/*
+  mov v.1d, x0
+  Cnt v1.8b, v.8b
+  Addv b2, v1.8b
+  Mov w0, v2.b[0]
+*/

Minor nit, but please use semicolons for MD file comments. The convention (at least in the aarch64 md files)
is to use ";;" at the beginning of the line. Also, please use uppercase letters for the assembly instructions
in the comment.

+(define_expand "popcount<mode>2"
+  [(match_operand:GPI 0 "register_operand")
+   (match_operand:GPI 1 "register_operand")]
+  "TARGET_SIMD"
+{
+  rtx v = gen_reg_rtx (V8QImode);
+  rtx v1 = gen_reg_rtx (V8QImode);
+  rtx r = gen_reg_rtx (QImode);
+  rtx in = operands[1];
+  rtx out = operands[0];
+  if(<MODE>mode == SImode)
+    {
+      rtx tmp;
+      tmp = gen_reg_rtx (DImode);
+      /* If we have SImode, zero extend to DImode, pop count does
+         not change if we have extra zeros. */
+      emit_insn (gen_zero_extendsidi2 (tmp, in));
+      in = tmp;
+    }
+  emit_move_insn (v, gen_lowpart (V8QImode, in));
+  emit_insn (gen_popcountv8qi2 (v1, v));
+  emit_insn (gen_reduc_plus_scal_v8qi (r, v1));
+  emit_insn (gen_zero_extendqi<mode>2 (out, r));
+  DONE;
+})
+
  (define_insn "clrsb<mode>2"
    [(set (match_operand:GPI 0 "register_operand" "=r")
          (clrsb:GPI (match_operand:GPI 1 "register_operand" "r")))]
diff --git a/gcc/testsuite/gcc.target/aarch64/popcount.c b/gcc/testsuite/gcc.target/aarch64/popcount.c
new file mode 100644
index 0000000..2d71168
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/popcount.c
@@ -0,0 +1,10 @@
+/* { dg-do compile } */
+/* { dg-options "-O2" } */
+
+int foo(int x)
+{
+  return __builtin_popcount(x);
+}
+
+/* { dg-final { scan-assembler-not "popcountdi2" } } */

__builtin_popcount takes an unsigned int, so this should be scanning for absence of popcountsi2 instead?
I suggest you also add a test for popcountdi2 using__builtin_popcountll.
Thanks,
Kyrill

+/* { dg-final { scan-assembler "cnt\t" } } */