From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 53053 invoked by alias); 8 Jun 2017 15:13:08 -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 53026 invoked by uid 89); 8 Jun 2017 15:13:06 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.9 required=5.0 tests=BAYES_00,SPF_PASS,T_RP_MATCHES_RCVD autolearn=ham version=3.3.2 spammy=00am 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; Thu, 08 Jun 2017 15:13:04 +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 B7329344; Thu, 8 Jun 2017 08:13:07 -0700 (PDT) Received: from e105689-lin.cambridge.arm.com (e105689-lin.cambridge.arm.com [10.2.207.32]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 54C193F3E1; Thu, 8 Jun 2017 08:13:06 -0700 (PDT) Subject: Re: [PATCH 4/6] Port prefetch configuration from aarch32 to aarch64 To: James Greenhalgh , Andrew Pinski Cc: Maxim Kuvyrkov , GCC Patches , Kyrylo Tkachov , Richard Guenther , nd@arm.com References: <4E4A2CDC-89F0-4CA4-804D-9B4442236276@linaro.org> <20170608134754.GA4880@arm.com> From: "Richard Earnshaw (lists)" Message-ID: Date: Thu, 08 Jun 2017 15:13:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.1.1 MIME-Version: 1.0 In-Reply-To: <20170608134754.GA4880@arm.com> Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: 7bit X-SW-Source: 2017-06/txt/msg00521.txt.bz2 On 08/06/17 14:47, James Greenhalgh wrote: > On Mon, Jan 30, 2017 at 08:35:00AM -0800, Andrew Pinski wrote: >> On Mon, Jan 30, 2017 at 3:48 AM, Maxim Kuvyrkov >> wrote: >>> This patch port prefetch configuration from aarch32 backend to aarch64. There is no code-generation change from this patch. >>> >>> This patch also happens to address Kyrill's comment on Andrew's prefetching patch at https://gcc.gnu.org/ml/gcc-patches/2017-01/msg02133.html . >>> >>> This patch also fixes a minor bug in aarch64_override_options_internal(), which used "selected_cpu->tune" instead of "aarch64_tune_params". >> >> I am not a fan of the macro at all. > > I'm with Andrew for this. The precedent in the AArch64 port is for > explicitly spelling this out, as we do with the branch costs, approx_modes, > vector costs etc. > > I'd rather we went that route than the macro you're using. I don't have > any objections to the rest of your patch. > > Thanks, > James > I disagree with having to write all these things out, but I do agree that we should be self-consistent within the port. The purpose of introducing the macros in the ARM port was to avoid the common problem of merging adding new parameters with adding new ports. C initializers for these tables assign values sequentially with zero-padding at the end, so failing to merge such changes carefully leads to real bugs in the compiler (even if just performance bugs). I notice that the last entry in the current tune params table is an int, rather than something that is type-checked (like the penultimate entry - an enum). Having the final entry be type checked at least ensures that the right number of elements exist, even if the order is not strictly checked. My 2p. R.