This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH 4/6] Port prefetch configuration from aarch32 to aarch64
- From: Maxim Kuvyrkov <maxim dot kuvyrkov at linaro dot org>
- To: "Richard Earnshaw (lists)" <Richard dot Earnshaw at arm dot com>
- Cc: James Greenhalgh <james dot greenhalgh at arm dot com>, Andrew Pinski <apinski at cavium dot com>, GCC Patches <gcc-patches at gcc dot gnu dot org>, Kyrylo Tkachov <kyrylo dot tkachov at arm dot com>, Richard Guenther <richard dot guenther at gmail dot com>, nd at arm dot com
- Date: Fri, 9 Jun 2017 10:30:02 +0300
- Subject: Re: [PATCH 4/6] Port prefetch configuration from aarch32 to aarch64
- Authentication-results: sourceware.org; auth=none
- References: <F7C2520D-866C-4293-831D-815BF466DFA2@linaro.org> <4E4A2CDC-89F0-4CA4-804D-9B4442236276@linaro.org> <CA+=Sn1nWwjnvMcYas9NBtnHe+NjAeQCch+RXJ+6sFR+Xyw_HyQ@mail.gmail.com> <20170608134754.GA4880@arm.com> <bc49a294-1cbd-1546-0509-f06df9dffe77@arm.com>
> On Jun 8, 2017, at 6:13 PM, Richard Earnshaw (lists) <Richard.Earnshaw@arm.com> wrote:
>
> 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
>>> <maxim.kuvyrkov@linaro.org> 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.
I'm re-writing the patch with approach that James suggested, and instead of AARCH64_PREFETCH_NOT_BENEFICIAL there will be "&generic_prefetch_tune", so not much copy-paste of parameters.
>
> 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.
If you prefer, I can move "&generic_prefetch_tune" to the bottom of "struct tune_params", thus enabling type checking on struct pointer.
--
Maxim Kuvyrkov
www.linaro.org