This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] [AArch64] Implement popcount pattern
- From: James Greenhalgh <james dot greenhalgh at arm dot com>
- To: Kyrill Tkachov <kyrylo dot tkachov at foss dot arm dot com>
- Cc: "Hurugalawadi, Naveen" <Naveen dot Hurugalawadi at cavium dot com>, "gcc-patches at gcc dot gnu dot org" <gcc-patches at gcc dot gnu dot org>, "Pinski, Andrew" <Andrew dot Pinski at cavium dot com>, Marcus Shawcroft <marcus dot shawcroft at arm dot com>, Richard Earnshaw <Richard dot Earnshaw at arm dot com>, <nd at arm dot com>
- Date: Wed, 1 Feb 2017 13:56:36 +0000
- Subject: Re: [PATCH] [AArch64] Implement popcount pattern
- Authentication-results: sourceware.org; auth=none
- Authentication-results: spf=pass (sender IP is 217.140.96.140) smtp.mailfrom=arm.com; cavium.com; dkim=none (message not signed) header.d=none;cavium.com; dmarc=bestguesspass action=none header.from=arm.com;cavium.com; dkim=none (message not signed) header.d=none;
- Nodisclaimer: True
- References: <CO2PR07MB26944CDE12E84FD22A41F68583870@CO2PR07MB2694.namprd07.prod.outlook.com> <CO2PR07MB2694C853BCFC5E83EF1E3D3583980@CO2PR07MB2694.namprd07.prod.outlook.com> <584E81A9.8000402@foss.arm.com> <SN2PR07MB27028DC0EDBA36D4C0E9671E839B0@SN2PR07MB2702.namprd07.prod.outlook.com> <584FE2A8.2080908@foss.arm.com>
- Spamdiagnosticmetadata: NSPM
- Spamdiagnosticoutput: 1:99
On Tue, Dec 13, 2016 at 11:59:36AM +0000, Kyrill Tkachov wrote:
> 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.
I didn't see a follow-up patch posted with Kyrill's comments addressed?
Thanks,
James