From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 130194 invoked by alias); 3 May 2017 09:39:18 -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 130177 invoked by uid 89); 3 May 2017 09:39:18 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-0.9 required=5.0 tests=BAYES_00,KAM_LAZY_DOMAIN_SECURITY,RP_MATCHES_RCVD autolearn=no version=3.3.2 spammy=adversely, sidi 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; Wed, 03 May 2017 09:39:07 +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 BE39E142F; Wed, 3 May 2017 02:39:08 -0700 (PDT) Received: from [10.2.206.52] (usa-sjc-imap-foss1.foss.arm.com [10.72.51.249]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 088EF3F4FF; Wed, 3 May 2017 02:39:07 -0700 (PDT) Subject: Re: [PATCH, GCC/ARM, stage4] Set mode for success result of atomic compare and swap To: Kyrill Tkachov , Ramana Radhakrishnan , Richard Earnshaw , "gcc-patches@gcc.gnu.org" References: <58F7674C.6050006@foss.arm.com> From: Thomas Preudhomme Message-ID: <8a356417-3ccd-f6c7-ee69-7b19719b4c44@foss.arm.com> Date: Wed, 03 May 2017 09:40:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.8.0 MIME-Version: 1.0 In-Reply-To: <58F7674C.6050006@foss.arm.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit X-IsSubscribed: yes X-SW-Source: 2017-05/txt/msg00181.txt.bz2 Hi Kyrill, On 19/04/17 14:34, Kyrill Tkachov wrote: > Hi Thomas, > > On 12/04/17 09:59, Thomas Preudhomme wrote: >> Hi, >> >> Currently atomic_compare_and_swap_1 define_insn do not have a mode >> set for the destination of the set indicating the success result of the >> instruction. This is because the operand can be either a CC_Z register >> (for 32-bit targets) or a SI register (for 16-bit Thumb targets). This >> result in lack of checking for the mode. >> >> This commit use a new CCSI iterator to solve this issue while avoiding >> duplication of the patterns. The insn name are kept unique by using >> attributes tied to the iterator (SIDI:mode and CCSI:arch) instead of >> usign the builtin mode attribute. Expander arm_expand_compare_and_swap >> is also adapted accordingly. >> >> ChangeLog entry is as follows: >> >> *** gcc/ChangeLog *** >> >> 2017-04-11 Thomas Preud'homme >> >> * config/arm/iterators.md (CCSI): New mode iterator. >> (arch): New mode attribute. >> * config/arm/sync.md (atomic_compare_and_swap_1): Rename into ... >> (atomic_compare_and_swap_1): This and ... >> (atomic_compare_and_swap_1): This. Use CCSI >> code iterator for success result mode. >> * config/arm/arm.c (arm_expand_compare_and_swap): Adapt code to use >> the corresponding new insn generators. >> >> Testing: arm-none-eabi cross-compiler built successfully for ARMv8-M >> Mainline and Baseline without the lack of destination mode warning in >> sync.md. Testsuite show no regression. >> > > Thanks for fixing these warnings. > The code looks ok to me but > I'd like to make sure that the rest of the arm atomic targets are not adversely > affected, > so please also do a test run for ARMv7-A and ARMv8-A targets. > Also, a bootstrap is required as always. Hi Kyrill, Bootstrapped and ran the testsuite for both ARMv7-A and ARMv8-A in both ARM and Thumb mode without any regression. I've also verified that a number of atomic related testcases [1][2] get the same code generation for ARMv7-A in ARM and Thumb mode as well as ARMv8-M Baseline. [1] For ARMv7-A ARM and Thumb mode, the following testcases were considered: gcc/testsuite/gcc.dg/atomic-compare-exchange-1.c gcc/testsuite/gcc.dg/atomic-compare-exchange-2.c gcc/testsuite/gcc.dg/atomic-compare-exchange-3.c gcc/testsuite/gcc.dg/atomic-exchange-1.c gcc/testsuite/gcc.dg/atomic-exchange-2.c gcc/testsuite/gcc.dg/atomic-exchange-3.c gcc/testsuite/gcc.dg/atomic-fence.c gcc/testsuite/gcc.dg/atomic-flag.c gcc/testsuite/gcc.dg/atomic-generic.c gcc/testsuite/gcc.dg/atomic-generic-aux.c gcc/testsuite/gcc.dg/atomic-invalid-2.c gcc/testsuite/gcc.dg/atomic-load-1.c gcc/testsuite/gcc.dg/atomic-load-2.c gcc/testsuite/gcc.dg/atomic-load-3.c gcc/testsuite/gcc.dg/atomic-lockfree.c gcc/testsuite/gcc.dg/atomic-lockfree-aux.c gcc/testsuite/gcc.dg/atomic-noinline.c gcc/testsuite/gcc.dg/atomic-noinline-aux.c gcc/testsuite/gcc.dg/atomic-op-1.c gcc/testsuite/gcc.dg/atomic-op-2.c gcc/testsuite/gcc.dg/atomic-op-3.c gcc/testsuite/gcc.dg/atomic-op-6.c gcc/testsuite/gcc.dg/atomic-store-1.c gcc/testsuite/gcc.dg/atomic-store-2.c gcc/testsuite/gcc.dg/atomic-store-3.c gcc/testsuite/g++.dg/ext/atomic-1.C gcc/testsuite/g++.dg/ext/atomic-2.C gcc/testsuite/gcc.target/arm/atomic-comp-swap-release-acquire-1.c gcc/testsuite/gcc.target/arm/atomic-op-acq_rel-1.c gcc/testsuite/gcc.target/arm/atomic-op-acquire-1.c gcc/testsuite/gcc.target/arm/atomic-op-char-1.c gcc/testsuite/gcc.target/arm/atomic-op-consume-1.c gcc/testsuite/gcc.target/arm/atomic-op-int-1.c gcc/testsuite/gcc.target/arm/atomic-op-relaxed-1.c gcc/testsuite/gcc.target/arm/atomic-op-release-1.c gcc/testsuite/gcc.target/arm/atomic-op-seq_cst-1.c gcc/testsuite/gcc.target/arm/atomic-op-short-1.c gcc/testsuite/gcc.target/arm/atomic_loaddi_1.c gcc/testsuite/gcc.target/arm/atomic_loaddi_4.c gcc/testsuite/gcc.target/arm/atomic_loaddi_7.c gcc/testsuite/gcc.target/arm/sync-1.c gcc/testsuite/gcc.target/arm/synchronize.c gcc/testsuite/gcc.target/arm/armv8-sync-comp-swap.c gcc/testsuite/gcc.target/arm/armv8-sync-op-acquire.c gcc/testsuite/gcc.target/arm/armv8-sync-op-full.c gcc/testsuite/gcc.target/arm/armv8-sync-op-release.c libstdc++-v3/testsuite/29_atomics/atomic/60658.cc libstdc++-v3/testsuite/29_atomics/atomic/62259.cc libstdc++-v3/testsuite/29_atomics/atomic/64658.cc libstdc++-v3/testsuite/29_atomics/atomic/65147.cc libstdc++-v3/testsuite/29_atomics/atomic/65913.cc libstdc++-v3/testsuite/29_atomics/atomic/70766.cc libstdc++-v3/testsuite/29_atomics/atomic/cons/49445.cc libstdc++-v3/testsuite/29_atomics/atomic/cons/constexpr.cc libstdc++-v3/testsuite/29_atomics/atomic/cons/copy_list.cc libstdc++-v3/testsuite/29_atomics/atomic/cons/default.cc libstdc++-v3/testsuite/29_atomics/atomic/cons/direct_list.cc libstdc++-v3/testsuite/29_atomics/atomic/cons/single_value.cc libstdc++-v3/testsuite/29_atomics/atomic/cons/user_pod.cc libstdc++-v3/testsuite/29_atomics/atomic/operators/51811.cc libstdc++-v3/testsuite/29_atomics/atomic/operators/56011.cc libstdc++-v3/testsuite/29_atomics/atomic/operators/integral_assignment.cc libstdc++-v3/testsuite/29_atomics/atomic/operators/integral_conversion.cc libstdc++-v3/testsuite/29_atomics/atomic/operators/pointer_partial_void.cc libstdc++-v3/testsuite/29_atomics/atomic/requirements/base_classes.cc libstdc++-v3/testsuite/29_atomics/atomic/requirements/compare_exchange_lowering.cc libstdc++-v3/testsuite/29_atomics/atomic/requirements/explicit_instantiation/1.cc libstdc++-v3/testsuite/29_atomics/atomic_flag/clear/1.cc libstdc++-v3/testsuite/29_atomics/atomic_flag/cons/1.cc libstdc++-v3/testsuite/29_atomics/atomic_flag/cons/56012.cc libstdc++-v3/testsuite/29_atomics/atomic_flag/cons/aggregate.cc libstdc++-v3/testsuite/29_atomics/atomic_flag/cons/default.cc libstdc++-v3/testsuite/29_atomics/atomic_flag/requirements/standard_layout.cc libstdc++-v3/testsuite/29_atomics/atomic_flag/requirements/trivial.cc libstdc++-v3/testsuite/29_atomics/atomic_flag/test_and_set/explicit.cc libstdc++-v3/testsuite/29_atomics/atomic_flag/test_and_set/implicit.cc libstdc++-v3/testsuite/29_atomics/atomic_integral/60940.cc libstdc++-v3/testsuite/29_atomics/atomic_integral/65147.cc libstdc++-v3/testsuite/29_atomics/atomic_integral/cons/constexpr.cc libstdc++-v3/testsuite/29_atomics/atomic_integral/cons/copy_list.cc libstdc++-v3/testsuite/29_atomics/atomic_integral/cons/default.cc libstdc++-v3/testsuite/29_atomics/atomic_integral/cons/direct_list.cc libstdc++-v3/testsuite/29_atomics/atomic_integral/cons/single_value.cc libstdc++-v3/testsuite/29_atomics/atomic_integral/operators/bitwise.cc libstdc++-v3/testsuite/29_atomics/atomic_integral/operators/decrement.cc libstdc++-v3/testsuite/29_atomics/atomic_integral/operators/increment.cc libstdc++-v3/testsuite/29_atomics/atomic_integral/operators/integral_assignment.cc libstdc++-v3/testsuite/29_atomics/atomic_integral/operators/integral_conversion.cc libstdc++-v3/testsuite/29_atomics/atomic_integral/requirements/standard_layout.cc libstdc++-v3/testsuite/29_atomics/atomic_integral/requirements/trivial.cc libstdc++-v3/testsuite/29_atomics/headers/atomic/functions_std_c++0x.cc libstdc++-v3/testsuite/29_atomics/headers/atomic/macros.cc libstdc++-v3/testsuite/29_atomics/headers/atomic/types_std_c++0x.cc [2] For ARMv8-M Baseline, the following testcases were considered: gcc/testsuite/gcc.dg/atomic-compare-exchange-1.c gcc/testsuite/gcc.dg/atomic-compare-exchange-2.c gcc/testsuite/gcc.dg/atomic-compare-exchange-3.c gcc/testsuite/gcc.dg/atomic-exchange-1.c gcc/testsuite/gcc.dg/atomic-exchange-2.c gcc/testsuite/gcc.dg/atomic-exchange-3.c gcc/testsuite/gcc.dg/atomic-fence.c gcc/testsuite/gcc.dg/atomic-flag.c gcc/testsuite/gcc.dg/atomic-generic.c gcc/testsuite/gcc.dg/atomic-generic-aux.c gcc/testsuite/gcc.dg/atomic-invalid-2.c gcc/testsuite/gcc.dg/atomic-load-1.c gcc/testsuite/gcc.dg/atomic-load-2.c gcc/testsuite/gcc.dg/atomic-load-3.c gcc/testsuite/gcc.dg/atomic-lockfree.c gcc/testsuite/gcc.dg/atomic-lockfree-aux.c gcc/testsuite/gcc.dg/atomic-noinline.c gcc/testsuite/gcc.dg/atomic-noinline-aux.c gcc/testsuite/gcc.dg/atomic-op-1.c gcc/testsuite/gcc.dg/atomic-op-2.c gcc/testsuite/gcc.dg/atomic-op-3.c gcc/testsuite/gcc.dg/atomic-op-6.c gcc/testsuite/gcc.dg/atomic-store-1.c gcc/testsuite/gcc.dg/atomic-store-2.c gcc/testsuite/gcc.dg/atomic-store-3.c gcc/testsuite/g++.dg/ext/atomic-1.C gcc/testsuite/g++.dg/ext/atomic-2.C gcc/testsuite/gcc.target/arm/atomic-comp-swap-release-acquire-3.c gcc/testsuite/gcc.target/arm/atomic-op-acq_rel-3.c gcc/testsuite/gcc.target/arm/atomic-op-acquire-3.c gcc/testsuite/gcc.target/arm/atomic-op-char-3.c gcc/testsuite/gcc.target/arm/atomic-op-consume-3.c gcc/testsuite/gcc.target/arm/atomic-op-int-3.c gcc/testsuite/gcc.target/arm/atomic-op-relaxed-3.c gcc/testsuite/gcc.target/arm/atomic-op-release-3.c gcc/testsuite/gcc.target/arm/atomic-op-seq_cst-3.c gcc/testsuite/gcc.target/arm/atomic-op-short-3.c gcc/testsuite/gcc.target/arm/sync-1.c gcc/testsuite/gcc.target/arm/synchronize.c gcc/testsuite/gcc.target/arm/armv8-sync-comp-swap.c gcc/testsuite/gcc.target/arm/armv8-sync-op-acquire.c gcc/testsuite/gcc.target/arm/armv8-sync-op-full.c gcc/testsuite/gcc.target/arm/armv8-sync-op-release.c libstdc++-v3/testsuite/29_atomics/atomic/60658.cc libstdc++-v3/testsuite/29_atomics/atomic/62259.cc libstdc++-v3/testsuite/29_atomics/atomic/64658.cc libstdc++-v3/testsuite/29_atomics/atomic/65147.cc libstdc++-v3/testsuite/29_atomics/atomic/65913.cc libstdc++-v3/testsuite/29_atomics/atomic/70766.cc libstdc++-v3/testsuite/29_atomics/atomic/cons/49445.cc libstdc++-v3/testsuite/29_atomics/atomic/cons/constexpr.cc libstdc++-v3/testsuite/29_atomics/atomic/cons/copy_list.cc libstdc++-v3/testsuite/29_atomics/atomic/cons/default.cc libstdc++-v3/testsuite/29_atomics/atomic/cons/direct_list.cc libstdc++-v3/testsuite/29_atomics/atomic/cons/single_value.cc libstdc++-v3/testsuite/29_atomics/atomic/cons/user_pod.cc libstdc++-v3/testsuite/29_atomics/atomic/operators/51811.cc libstdc++-v3/testsuite/29_atomics/atomic/operators/56011.cc libstdc++-v3/testsuite/29_atomics/atomic/operators/integral_assignment.cc libstdc++-v3/testsuite/29_atomics/atomic/operators/integral_conversion.cc libstdc++-v3/testsuite/29_atomics/atomic/operators/pointer_partial_void.cc libstdc++-v3/testsuite/29_atomics/atomic/requirements/base_classes.cc libstdc++-v3/testsuite/29_atomics/atomic/requirements/compare_exchange_lowering.cc libstdc++-v3/testsuite/29_atomics/atomic/requirements/explicit_instantiation/1.cc libstdc++-v3/testsuite/29_atomics/atomic_flag/clear/1.cc libstdc++-v3/testsuite/29_atomics/atomic_flag/cons/1.cc libstdc++-v3/testsuite/29_atomics/atomic_flag/cons/56012.cc libstdc++-v3/testsuite/29_atomics/atomic_flag/cons/aggregate.cc libstdc++-v3/testsuite/29_atomics/atomic_flag/cons/default.cc libstdc++-v3/testsuite/29_atomics/atomic_flag/requirements/standard_layout.cc libstdc++-v3/testsuite/29_atomics/atomic_flag/requirements/trivial.cc libstdc++-v3/testsuite/29_atomics/atomic_flag/test_and_set/explicit.cc libstdc++-v3/testsuite/29_atomics/atomic_flag/test_and_set/implicit.cc libstdc++-v3/testsuite/29_atomics/atomic_integral/60940.cc libstdc++-v3/testsuite/29_atomics/atomic_integral/65147.cc libstdc++-v3/testsuite/29_atomics/atomic_integral/cons/constexpr.cc libstdc++-v3/testsuite/29_atomics/atomic_integral/cons/copy_list.cc libstdc++-v3/testsuite/29_atomics/atomic_integral/cons/default.cc libstdc++-v3/testsuite/29_atomics/atomic_integral/cons/direct_list.cc libstdc++-v3/testsuite/29_atomics/atomic_integral/cons/single_value.cc libstdc++-v3/testsuite/29_atomics/atomic_integral/operators/bitwise.cc libstdc++-v3/testsuite/29_atomics/atomic_integral/operators/decrement.cc libstdc++-v3/testsuite/29_atomics/atomic_integral/operators/increment.cc libstdc++-v3/testsuite/29_atomics/atomic_integral/operators/integral_assignment.cc libstdc++-v3/testsuite/29_atomics/atomic_integral/operators/integral_conversion.cc libstdc++-v3/testsuite/29_atomics/atomic_integral/requirements/standard_layout.cc libstdc++-v3/testsuite/29_atomics/atomic_integral/requirements/trivial.cc libstdc++-v3/testsuite/29_atomics/headers/atomic/functions_std_c++0x.cc libstdc++-v3/testsuite/29_atomics/headers/atomic/macros.cc libstdc++-v3/testsuite/29_atomics/headers/atomic/types_std_c++0x.cc > > Ok with that testing. Just to make sure, canyou confirm again that you're Ok for this to be commited in trunk with that amount of testing now that GCC 7.1 is released? Best regards, Thomas