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
