> On Jun 8, 2017, at 6:13 PM, Richard Earnshaw (lists) > <richard.earns...@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.kuvyr...@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