From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 83693 invoked by alias); 19 Aug 2015 12:59:06 -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 83677 invoked by uid 89); 19 Aug 2015 12:59:05 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.3 required=5.0 tests=AWL,BAYES_00,FREEMAIL_FROM,RCVD_IN_DNSWL_LOW,SPF_PASS autolearn=ham version=3.3.2 X-HELO: mail-la0-f51.google.com Received: from mail-la0-f51.google.com (HELO mail-la0-f51.google.com) (209.85.215.51) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-GCM-SHA256 encrypted) ESMTPS; Wed, 19 Aug 2015 12:59:04 +0000 Received: by laba3 with SMTP id a3so2441285lab.1 for ; Wed, 19 Aug 2015 05:59:01 -0700 (PDT) MIME-Version: 1.0 X-Received: by 10.153.8.137 with SMTP id dk9mr11200655lad.57.1439989141133; Wed, 19 Aug 2015 05:59:01 -0700 (PDT) Received: by 10.25.158.69 with HTTP; Wed, 19 Aug 2015 05:59:01 -0700 (PDT) In-Reply-To: <20150819113926.GA3411@e107456-lin.cambridge.arm.com> References: <20150819113926.GA3411@e107456-lin.cambridge.arm.com> Date: Wed, 19 Aug 2015 13:00:00 -0000 Message-ID: Subject: Re: [PATCH/AARCH64] Remove index from AARCH64_FUSION_PAIR From: Andrew Pinski To: James Greenhalgh Cc: GCC Patches Content-Type: text/plain; charset=UTF-8 X-IsSubscribed: yes X-SW-Source: 2015-08/txt/msg01077.txt.bz2 On Wed, Aug 19, 2015 at 7:39 PM, James Greenhalgh wrote: > 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? Yes I actually thought of that after I had submitted the patch. > > If so, could you respin with that change? Respinning this patch and the one for AARCH64_EXTRA_TUNING_OPTION. Thanks, Andrew > > 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[] = >> { >