From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 125875 invoked by alias); 19 Aug 2015 11:39:33 -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 125864 invoked by uid 89); 19 Aug 2015 11:39:32 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.4 required=5.0 tests=AWL,BAYES_00,RP_MATCHES_RCVD,SPF_PASS autolearn=ham version=3.3.2 X-HELO: cam-smtp0.cambridge.arm.com Received: from fw-tnat.cambridge.arm.com (HELO cam-smtp0.cambridge.arm.com) (217.140.96.140) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-SHA encrypted) ESMTPS; Wed, 19 Aug 2015 11:39:30 +0000 Received: from e107456-lin.cambridge.arm.com (e107456-lin.cambridge.arm.com [10.2.207.14]) by cam-smtp0.cambridge.arm.com (8.13.8/8.13.8) with ESMTP id t7JBdQQd028309; Wed, 19 Aug 2015 12:39:26 +0100 Date: Wed, 19 Aug 2015 12:11:00 -0000 From: James Greenhalgh To: Andrew Pinski Cc: GCC Patches Subject: Re: [PATCH/AARCH64] Remove index from AARCH64_FUSION_PAIR Message-ID: <20150819113926.GA3411@e107456-lin.cambridge.arm.com> References: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.21 (2010-09-15) X-IsSubscribed: yes X-SW-Source: 2015-08/txt/msg01065.txt.bz2 On Wed, Aug 19, 2015 at 12:11:04PM +0100, Andrew Pinski wrote: > Instead of doing an explicit index in aarch64-fusion-pairs.def, we > should have an enum which does the index instead. This allows > you to add/remove them without worrying about the order being > correct and having holes or worry about merge conflicts. > > OK? Bootstrapped and tested on aarch64-linux-gnu with no regressions. > Looks good, it would be good to expand this patch to get rid of the messy way we build the AARCH64_FUSE_ALL macro: > /* Hacky macro to build AARCH64_FUSE_ALL. The sequence below expands > to: > AARCH64_FUSE_ALL = 0 | AARCH64_FUSE_index1 | AARCH64_FUSE_index2 ... */ > #undef AARCH64_FUSION_PAIR > #define AARCH64_FUSION_PAIR(x, name, y) \ > | AARCH64_FUSE_##name > > AARCH64_FUSE_ALL = 0 > #include "aarch64-fusion-pairs.def" We should now be able to do something like: AARCH64_FUSE_ALL = ((1 << AARCH64_FUSE_index_END) - 1) Right? If so, could you respin with that change? Thanks, James > diff --git a/gcc/config/aarch64/aarch64-fusion-pairs.def b/gcc/config/aarch64/aarch64-fusion-pairs.def > index a7b00f6..53bbef4 100644 > --- a/gcc/config/aarch64/aarch64-fusion-pairs.def > +++ b/gcc/config/aarch64/aarch64-fusion-pairs.def > @@ -20,19 +20,17 @@ > /* Pairs of instructions which can be fused. before including this file, > define a macro: > > - AARCH64_FUSION_PAIR (name, internal_name, index_bit) > + AARCH64_FUSION_PAIR (name, internal_name) > > Where: > > NAME is a string giving a friendly name for the instructions to fuse. > INTERNAL_NAME gives the internal name suitable for appending to > - AARCH64_FUSE_ to give an enum name. > - INDEX_BIT is the bit to set in the bitmask of supported fusion > - operations. */ > - > -AARCH64_FUSION_PAIR ("mov+movk", MOV_MOVK, 0) > -AARCH64_FUSION_PAIR ("adrp+add", ADRP_ADD, 1) > -AARCH64_FUSION_PAIR ("movk+movk", MOVK_MOVK, 2) > -AARCH64_FUSION_PAIR ("adrp+ldr", ADRP_LDR, 3) > -AARCH64_FUSION_PAIR ("cmp+branch", CMP_BRANCH, 4) > + AARCH64_FUSE_ to give an enum name. */ > + > +AARCH64_FUSION_PAIR ("mov+movk", MOV_MOVK) > +AARCH64_FUSION_PAIR ("adrp+add", ADRP_ADD) > +AARCH64_FUSION_PAIR ("movk+movk", MOVK_MOVK) > +AARCH64_FUSION_PAIR ("adrp+ldr", ADRP_LDR) > +AARCH64_FUSION_PAIR ("cmp+branch", CMP_BRANCH) > > diff --git a/gcc/config/aarch64/aarch64-protos.h b/gcc/config/aarch64/aarch64-protos.h > index 0b09d49..c4c1817 100644 > --- a/gcc/config/aarch64/aarch64-protos.h > +++ b/gcc/config/aarch64/aarch64-protos.h > @@ -201,8 +201,18 @@ struct tune_params > unsigned int extra_tuning_flags; > }; > > -#define AARCH64_FUSION_PAIR(x, name, index) \ > - AARCH64_FUSE_##name = (1 << index), > +#define AARCH64_FUSION_PAIR(x, name) \ > + AARCH64_FUSE_##name##_index, > +/* Supported fusion operations. */ > +enum aarch64_fusion_pairs_index > +{ > +#include "aarch64-fusion-pairs.def" > + AARCH64_FUSE_index_END > +}; > +#undef AARCH64_FUSION_PAIR > + > +#define AARCH64_FUSION_PAIR(x, name) \ > + AARCH64_FUSE_##name = (1 << AARCH64_FUSE_##name##_index), > /* Supported fusion operations. */ > enum aarch64_fusion_pairs > { > @@ -213,7 +223,7 @@ enum aarch64_fusion_pairs > to: > AARCH64_FUSE_ALL = 0 | AARCH64_FUSE_index1 | AARCH64_FUSE_index2 ... */ > #undef AARCH64_FUSION_PAIR > -#define AARCH64_FUSION_PAIR(x, name, y) \ > +#define AARCH64_FUSION_PAIR(x, name) \ > | AARCH64_FUSE_##name > > AARCH64_FUSE_ALL = 0 > diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c > index aa268ae..162e25e 100644 > --- a/gcc/config/aarch64/aarch64.c > +++ b/gcc/config/aarch64/aarch64.c > @@ -172,7 +172,7 @@ struct aarch64_flag_desc > unsigned int flag; > }; > > -#define AARCH64_FUSION_PAIR(name, internal_name, y) \ > +#define AARCH64_FUSION_PAIR(name, internal_name) \ > { name, AARCH64_FUSE_##internal_name }, > static const struct aarch64_flag_desc aarch64_fusible_pairs[] = > {