From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 104045 invoked by alias); 9 Feb 2016 16:27:07 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 104033 invoked by uid 89); 9 Feb 2016 16:27:07 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.2 required=5.0 tests=BAYES_00,KAM_LAZY_DOMAIN_SECURITY,RP_MATCHES_RCVD autolearn=no version=3.3.2 spammy=hunk, Prototype, PASS, tabs X-HELO: foss.arm.com Received: from foss.arm.com (HELO foss.arm.com) (217.140.101.70) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 09 Feb 2016 16:27:06 +0000 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.72.51.249]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id E683749; Tue, 9 Feb 2016 08:26:17 -0800 (PST) Received: from [10.2.206.200] (e100706-lin.cambridge.arm.com [10.2.206.200]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 043303F25E; Tue, 9 Feb 2016 08:27:03 -0800 (PST) Message-ID: <56BA1356.4060506@foss.arm.com> Date: Tue, 09 Feb 2016 16:27:00 -0000 From: Kyrill Tkachov User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.2.0 MIME-Version: 1.0 To: Michael Collison , gcc Patches , Ramana Radhakrishnan Subject: Re: [ARM] Use vector wide add for mixed-mode adds References: <565BA3CC.3050800@linaro.org> <566995BE.8040206@arm.com> <5671FB8A.4000004@linaro.org> In-Reply-To: <5671FB8A.4000004@linaro.org> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit X-SW-Source: 2016-02/txt/msg00629.txt.bz2 Hi Michael, On 17/12/15 00:02, Michael Collison wrote: > Kyrill, > > I have attached a patch that address your comments. The only change I would ask you to re-consider renaming is the function 'bool aarch32_simd_check_vect_par_cnst_half'. This function was copied from the aarch64 port and I thought it as > important to match the naming for maintenance purposes. I did rename the function to 'bool arm_simd_check_vect_par_cnst_half_p'. I changed 'aarch32' to 'arm' and added '_p' per you suggestions. Is this okay? > Ok, that's fine with me. > I implemented all your other change suggestions. > Thanks, sorry it took a long time to get back to this, I was busy with regression-fixing patches as we're in bug-fixing mode... > 2015-12-16 Michael Collison > > * config/arm/neon.md (widen_sum): New patterns where > mode is VQI to improve mixed mode vectorization. > * config/arm/neon.md (vec_sel_widen_ssum_lo3): New > define_insn to match low half of signed vaddw. > * config/arm/neon.md (vec_sel_widen_ssum_hi3): New > define_insn to match high half of signed vaddw. > * config/arm/neon.md (vec_sel_widen_usum_lo3): New > define_insn to match low half of unsigned vaddw. > * config/arm/neon.md (vec_sel_widen_usum_hi3): New > define_insn to match high half of unsigned vaddw. > * config/arm/arm.c (arm_simd_vect_par_cnst_half): New function. > (arm_simd_check_vect_par_cnst_half_p): Likewise. > * config/arm/arm-protos.h (arm_simd_vect_par_cnst_half): Prototype > for new function. > (arm_simd_check_vect_par_cnst_half_p): Likewise. > * config/arm/predicates.md (vect_par_constant_high): Support > big endian and simplify by calling > arm_simd_check_vect_par_cnst_half > (vect_par_constant_low): Likewise. > * testsuite/gcc.target/arm/neon-vaddws16.c: New test. > * testsuite/gcc.target/arm/neon-vaddws32.c: New test. > * testsuite/gcc.target/arm/neon-vaddwu16.c: New test. > * testsuite/gcc.target/arm/neon-vaddwu32.c: New test. > * testsuite/gcc.target/arm/neon-vaddwu8.c: New test. > * testsuite/lib/target-supports.exp > (check_effective_target_vect_widen_sum_hi_to_si_pattern): Indicate > that arm neon support vector widen sum of HImode TO SImode. > I've tried this out and I have a few comments. The arm.c hunk doesn't apply to current trunk anymore due to context. Can you please rebase the patch? I've fixed it up manually in my tree so I can build it. With this patch I'm seeing two PASS->FAIL on arm-none-eabi: FAIL: gcc.dg/vect/slp-reduc-3.c -flto -ffat-lto-objects scan-tree-dump-times vect "vectorizing stmts using SLP" 1 FAIL: gcc.dg/vect/slp-reduc-3.c scan-tree-dump-times vect "vectorizing stmts using SLP" 1 My compiler is configured with --with-float=hard --with-cpu=cortex-a9 --with-fpu=neon --with-mode=thumb Can you please look into these? Maybe it's just the tests that need adjustment? Also, I'm seeing the new tests give an error: ERROR: gcc.target/arm/neon-vaddws16.c: Unrecognized option type: arm_neon_ok for " dg-add-options 3 arm_neon_ok " UNRESOLVED: gcc.target/arm/neon-vaddws16.c: Unrecognized option type: arm_neon_ok for " dg-add-options 3 arm_neon_ok " That've because the dg-add-options argument should be arm_neon rather than arm_neon_ok. Also, since the new tests are compile-only the effective target check should be arm_neon_ok rather than arm_neon_hw. I also see ./contrib/check_GNU_style.sh complaining about some minor style issues like trailing whitespace and blocks of whitespace that should be replaced with tabs. In any case, this patch is GCC 7 material at this point, so I think with the above issues resolved (and the FAILs investigated) this should be in good shape. Thanks, Kyrill