Re: [Patch AArch64] Switch constant pools to separate rodata sections.

[James Greenhalgh <james dot greenhalgh at arm dot com>]

On Tue, Nov 03, 2015 at 04:36:45PM +0000, Ramana Radhakrishnan wrote:
> Hi,
> 
> 	Now that PR63304 is fixed and we have an option to address
> any part of the memory using adrp / add or adrp / ldr instructions
> it makes sense to switch out literal pools into their own
> mergeable sections by default.
> 
> This would mean that by default we could now start getting
> the benefits of constant sharing across the board, potentially
> improving code size. The other advantage of doing so, for the
> security conscious is that this prevents intermingling of literal
> pools with code.
> 
> Wilco's kindly done some performance measurements and suggests that
> there is not really a performance regression in doing this.
> I've looked at the code size for SPEC2k6 today at -Ofast and
> in general there is a good code size improvement as expected
> by sharing said constants.
> 
> Tested on aarch64-none-elf with no regressions and bootstrapped 
> and regression tested in my tree for a number of days now.
> 
> Ok to commit ?

OK with the comment nits below fixed up.

>    * config/aarch64/aarch64.c (aarch64_select_rtx_section): Switch
> to default section handling for non PC relative literal loads.

> diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
> index 5c8604f..9d709e5 100644
> --- a/gcc/config/aarch64/aarch64.c
> +++ b/gcc/config/aarch64/aarch64.c
> @@ -5244,13 +5244,22 @@ aarch64_use_blocks_for_constant_p (machine_mode mode ATTRIBUTE_UNUSED,
>    return false;
>  }
>  
> +/* Force all constant pool entries into the current function section.

Is this comment accurate now? I think it only applied to -mcmodel=large
but maybe I'm misunderstanding?

> +   In the large model we cannot reliably address all the address space
> +   thus for now, inline this with the text.  */
>  static section *
> +aarch64_select_rtx_section (machine_mode mode,
> +			    rtx x,
> +			    unsigned HOST_WIDE_INT align)
> +{
> +  /* Force all constant pool entries into the current function section.
> +     In the large model we cannot reliably address all the address space
> +     thus for now, inline this with the text.  */
> +  if (!aarch64_nopcrelative_literal_loads
> +      || aarch64_cmodel == AARCH64_CMODEL_LARGE)
> +    return function_section (current_function_decl);

This is just a copy paste of the text above (and probably the better place
for it).

I think we'd want a more general comment at the top of the function,
then this can stay.

Thanks,
James