Hi Naveen,

On 13/12/16 11:51, Hurugalawadi, Naveen wrote:
Hi Kyrill,

Thanks for reviewing the patch and your useful comments.

looks good to me if it has gone through the normal required
bootstrap and testing, but I can't approve.
Bootstrapped and Regression Tested on aarch64-thunderx-linux.

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

__builtin_popcount takes an unsigned int, so this should be
scanning for absence of popcountsi2 instead?
Done.

Please find attached the modified patch as per review comments
and let me know if its okay for Stage-1 or current branch.

This looks much better, thanks.
I still have a minor nit about the testcase.

+long
+foo1 (int x)
+{
+  return __builtin_popcountl (x);
+}

On ILP32 systems this would still use the SImode patterns, so I suggest you use 
__builtin_popcountll and
an unsigned long long return type to ensure you always exercise the 64-bit code.

+
+/* { dg-final { scan-assembler-not "popcount" } } */


This looks ok to me otherwise, but you'll need an ok from the aarch64 folk.
Kyrill

Thanks,
Naveen

Reply via email to