Re: [PATCH 4/6] Port prefetch configuration from aarch32 to aarch64

["Richard Earnshaw (lists)" <Richard dot Earnshaw at arm dot com>]

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.

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.