On 03/11/15 17:09, James Greenhalgh wrote:
> 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.
>
True and I've just been reading more of the backend - We could now start using
blocks for constant pools as well. So let's do that.
How does something like this look ?
Tested on aarch64-none-elf - no regressions.
2015-11-04 Ramana Radhakrishnan <ramana.radhakrish...@arm.com>
* config/aarch64/aarch64.c
(aarch64_can_use_per_function_literal_pools_p): New.
(aarch64_use_blocks_for_constant_p): Adjust declaration
and use aarch64_can_use_function_literal_pools_p.
(aarch64_select_rtx_section): Update.
diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index 5c8604f..4f5e069 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -5235,24 +5235,37 @@ aarch64_uxt_size (int shift, HOST_WIDE_INT mask)
return 0;
}
+/* Constant pools are per function only when PC relative
+ literal loads are true or we are in the large memory
+ model. */
+static inline bool
+aarch64_can_use_per_function_literal_pools_p (void)
+{
+ return (!aarch64_nopcrelative_literal_loads
+ || aarch64_cmodel == AARCH64_CMODEL_LARGE);
+}
+
static bool
-aarch64_use_blocks_for_constant_p (machine_mode mode ATTRIBUTE_UNUSED,
- const_rtx x ATTRIBUTE_UNUSED)
+aarch64_use_blocks_for_constant_p (machine_mode, const_rtx)
{
/* We can't use blocks for constants when we're using a per-function
constant pool. */
- return false;
+ return !aarch64_can_use_per_function_literal_pools_p ();
}
+/* Select appropriate section for constants depending
+ on where we place literal pools. */
+
static section *
-aarch64_select_rtx_section (machine_mode mode ATTRIBUTE_UNUSED,
- rtx x ATTRIBUTE_UNUSED,
- unsigned HOST_WIDE_INT align ATTRIBUTE_UNUSED)
+aarch64_select_rtx_section (machine_mode mode,
+ rtx x,
+ unsigned HOST_WIDE_INT align)
{
- /* Force all constant pool entries into the current function section. */
- return function_section (current_function_decl);
-}
+ if (aarch64_can_use_per_function_literal_pools_p ())
+ return function_section (current_function_decl);
+ return default_elf_select_rtx_section (mode, x, align);
+}
/* Costs. */
