public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [Patch AArch64] Switch constant pools to separate rodata sections.
@ 2015-11-03 16:36 Ramana Radhakrishnan
  2015-11-03 17:10 ` James Greenhalgh
  0 siblings, 1 reply; 11+ messages in thread
From: Ramana Radhakrishnan @ 2015-11-03 16:36 UTC (permalink / raw)
  To: gcc-patches

[-- Attachment #1: Type: text/plain, Size: 1055 bytes --]

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 ?

regards
Ramana

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

[-- Attachment #2: default-rodata.txt --]
[-- Type: text/plain, Size: 1312 bytes --]

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.
+   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 ATTRIBUTE_UNUSED,
-			    rtx x ATTRIBUTE_UNUSED,
-			    unsigned HOST_WIDE_INT align ATTRIBUTE_UNUSED)
-{
-  /* Force all constant pool entries into the current function section.  */
-  return function_section (current_function_decl);
+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);
+
+  return default_elf_select_rtx_section (mode, x, align);
 }
 
 
-- 
1.9.1


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Patch AArch64] Switch constant pools to separate rodata sections.
  2015-11-03 16:36 [Patch AArch64] Switch constant pools to separate rodata sections Ramana Radhakrishnan
@ 2015-11-03 17:10 ` James Greenhalgh
  2015-11-04 14:26   ` Ramana Radhakrishnan
  0 siblings, 1 reply; 11+ messages in thread
From: James Greenhalgh @ 2015-11-03 17:10 UTC (permalink / raw)
  To: Ramana Radhakrishnan; +Cc: gcc-patches

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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Patch AArch64] Switch constant pools to separate rodata sections.
  2015-11-03 17:10 ` James Greenhalgh
@ 2015-11-04 14:26   ` Ramana Radhakrishnan
  2015-11-05  9:35     ` James Greenhalgh
                       ` (2 more replies)
  0 siblings, 3 replies; 11+ messages in thread
From: Ramana Radhakrishnan @ 2015-11-04 14:26 UTC (permalink / raw)
  To: James Greenhalgh; +Cc: gcc-patches

[-- Attachment #1: Type: text/plain, Size: 3143 bytes --]



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.radhakrishnan@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.

[-- Attachment #2: p1.txt --]
[-- Type: text/plain, Size: 1633 bytes --]

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.  */
 

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Patch AArch64] Switch constant pools to separate rodata sections.
  2015-11-04 14:26   ` Ramana Radhakrishnan
@ 2015-11-05  9:35     ` James Greenhalgh
  2015-11-08 11:43     ` Andreas Schwab
  2015-11-10 16:39     ` Alan Lawrence
  2 siblings, 0 replies; 11+ messages in thread
From: James Greenhalgh @ 2015-11-05  9:35 UTC (permalink / raw)
  To: Ramana Radhakrishnan; +Cc: gcc-patches

On Wed, Nov 04, 2015 at 02:26:31PM +0000, Ramana Radhakrishnan wrote:
> 
> 
> 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 ?

One tiny nit. Otherwise, this is OK.


> Tested on aarch64-none-elf - no regressions.
> 
> 2015-11-04  Ramana Radhakrishnan  <ramana.radhakrishnan@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.  */

Newline here please.

Thanks,
James

> +static inline bool
> +aarch64_can_use_per_function_literal_pools_p (void)
> +{
> +  return (!aarch64_nopcrelative_literal_loads
> +	  || aarch64_cmodel == AARCH64_CMODEL_LARGE);
> +}
> +

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Patch AArch64] Switch constant pools to separate rodata sections.
  2015-11-04 14:26   ` Ramana Radhakrishnan
  2015-11-05  9:35     ` James Greenhalgh
@ 2015-11-08 11:43     ` Andreas Schwab
  2015-11-09 14:24       ` Ramana Radhakrishnan
  2015-11-09 16:46       ` Ramana Radhakrishnan
  2015-11-10 16:39     ` Alan Lawrence
  2 siblings, 2 replies; 11+ messages in thread
From: Andreas Schwab @ 2015-11-08 11:43 UTC (permalink / raw)
  To: Ramana Radhakrishnan; +Cc: James Greenhalgh, gcc-patches

This is causing a bootstrap comparison failure in gcc/go/gogo.o.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Patch AArch64] Switch constant pools to separate rodata sections.
  2015-11-08 11:43     ` Andreas Schwab
@ 2015-11-09 14:24       ` Ramana Radhakrishnan
  2015-11-09 16:46       ` Ramana Radhakrishnan
  1 sibling, 0 replies; 11+ messages in thread
From: Ramana Radhakrishnan @ 2015-11-09 14:24 UTC (permalink / raw)
  To: Andreas Schwab; +Cc: James Greenhalgh, gcc-patches



On 08/11/15 11:42, Andreas Schwab wrote:
> This is causing a bootstrap comparison failure in gcc/go/gogo.o.

I'm looking into this - this is now PR68256.

regards
Ramana
> 
> Andreas.
> 

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Patch AArch64] Switch constant pools to separate rodata sections.
  2015-11-08 11:43     ` Andreas Schwab
  2015-11-09 14:24       ` Ramana Radhakrishnan
@ 2015-11-09 16:46       ` Ramana Radhakrishnan
  2015-11-09 17:21         ` James Greenhalgh
  1 sibling, 1 reply; 11+ messages in thread
From: Ramana Radhakrishnan @ 2015-11-09 16:46 UTC (permalink / raw)
  To: gcc-patches; +Cc: Andreas Schwab, James Greenhalgh

[-- Attachment #1: Type: text/plain, Size: 796 bytes --]



On 08/11/15 11:42, Andreas Schwab wrote:
> This is causing a bootstrap comparison failure in gcc/go/gogo.o.
> 
> Andreas.
> 

I've had a look at this for sometime this afternoon and the trigger is the aarch64_use_constant_blocks_p change which appears to be causing a bootstrap comparison failure because of differences to offsets in add instructions when built with debug and without debug. For now, in the interest of go bootstraps continuing on trunk - I'm proposing a patch that partially rolls back the change in aarch64_use_constant_blocks_p and will still look into the underlying issue.

Bootstrapped on aarch64-none-linux-gnu including (c,c++ and go) - testing finished ok.

Ok ?


Ramana


PR bootstrap/68256

* config/aarch64/aarch64.c (aarch64_use_constant_blocks_p): Return false.

[-- Attachment #2: go-workaround.txt --]
[-- Type: text/plain, Size: 770 bytes --]

diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index 1b7be83..1fff878 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -5251,9 +5251,11 @@ aarch64_can_use_per_function_literal_pools_p (void)
 static bool
 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 !aarch64_can_use_per_function_literal_pools_p ();
+  /* Fixme:: In an ideal world this would work similar
+     to the logic in aarch64_select_rtx_section but this
+     breaks bootstrap in gcc go.  For now we workaround
+     this by returning false here.  */
+  return false;
 }
 
 /* Select appropriate section for constants depending

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Patch AArch64] Switch constant pools to separate rodata sections.
  2015-11-09 16:46       ` Ramana Radhakrishnan
@ 2015-11-09 17:21         ` James Greenhalgh
  0 siblings, 0 replies; 11+ messages in thread
From: James Greenhalgh @ 2015-11-09 17:21 UTC (permalink / raw)
  To: Ramana Radhakrishnan; +Cc: gcc-patches, Andreas Schwab

On Mon, Nov 09, 2015 at 04:46:01PM +0000, Ramana Radhakrishnan wrote:
> 
> 
> On 08/11/15 11:42, Andreas Schwab wrote:
> > This is causing a bootstrap comparison failure in gcc/go/gogo.o.
> > 
> > Andreas.
> > 
> 
> I've had a look at this for sometime this afternoon and the trigger is the
> aarch64_use_constant_blocks_p change which appears to be causing a bootstrap
> comparison failure because of differences to offsets in add instructions when
> built with debug and without debug. For now, in the interest of go bootstraps
> continuing on trunk - I'm proposing a patch that partially rolls back the
> change in aarch64_use_constant_blocks_p and will still look into the
> underlying issue.
> 
> Bootstrapped on aarch64-none-linux-gnu including (c,c++ and go) - testing
> finished ok.
> 
> Ok ?

I agreem, this seems like the timely way to get the auto-testers back
building.

OK.

Thanks,
James

> 
> 
> Ramana
> 
> 
> PR bootstrap/68256
> 
> * config/aarch64/aarch64.c (aarch64_use_constant_blocks_p): Return false.

> diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
> index 1b7be83..1fff878 100644
> --- a/gcc/config/aarch64/aarch64.c
> +++ b/gcc/config/aarch64/aarch64.c
> @@ -5251,9 +5251,11 @@ aarch64_can_use_per_function_literal_pools_p (void)
>  static bool
>  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 !aarch64_can_use_per_function_literal_pools_p ();
> +  /* Fixme:: In an ideal world this would work similar
> +     to the logic in aarch64_select_rtx_section but this
> +     breaks bootstrap in gcc go.  For now we workaround
> +     this by returning false here.  */
> +  return false;
>  }
>  
>  /* Select appropriate section for constants depending

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Patch AArch64] Switch constant pools to separate rodata sections.
  2015-11-04 14:26   ` Ramana Radhakrishnan
  2015-11-05  9:35     ` James Greenhalgh
  2015-11-08 11:43     ` Andreas Schwab
@ 2015-11-10 16:39     ` Alan Lawrence
  2015-11-10 16:47       ` Ramana Radhakrishnan
  2015-11-10 17:00       ` Alan Lawrence
  2 siblings, 2 replies; 11+ messages in thread
From: Alan Lawrence @ 2015-11-10 16:39 UTC (permalink / raw)
  To: Ramana Radhakrishnan, James Greenhalgh; +Cc: gcc-patches

On 04/11/15 14:26, Ramana Radhakrishnan wrote:
>
> 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.radhakrishnan@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.
>

Since r229878, I've been seeing

FAIL: gcc.dg/attr-weakref-1.c (test for excess errors)
UNRESOLVED: gcc.dg/attr-weakref-1.c compilation failed to produce executable

(both previously passing) on aarch64-none-elf, aarch64_be-none-elf, and 
aarch64-none-linux-gnu. Here's a log from aarch64_be-none-elf (the others look 
similar):

/work/alalaw01/build-aarch64_be-none-elf/obj/gcc2/gcc/xgcc 
-B/work/alalaw01/build-aarch64_be-none-elf/obj/gcc2/gcc/ 
/work/alalaw01/src/gcc/gcc/testsuite/gcc.dg/attr-weakref-1.c 
-fno-diagnostics-show-caret -fdiagnostics-color=never -O2 
/work/alalaw01/src/gcc/gcc/testsuite/gcc.dg/attr-weakref-1a.c 
-specs=aem-validation.specs -lm -o ./attr-weakref-1.exe
/tmp/ccEfngi6.o:(.rodata.cst8+0x30): undefined reference to `wv12'
/tmp/ccEfngi6.o:(.rodata.cst8+0x38): undefined reference to `wv12'
/tmp/ccEfngi6.o:(.rodata.cst8+0x60): undefined reference to `wf12'
/tmp/ccEfngi6.o:(.rodata.cst8+0x68): undefined reference to `wf12'
collect2: error: ld returned 1 exit status
compiler exited with status 1
output is:
/tmp/ccEfngi6.o:(.rodata.cst8+0x30): undefined reference to `wv12'
/tmp/ccEfngi6.o:(.rodata.cst8+0x38): undefined reference to `wv12'
/tmp/ccEfngi6.o:(.rodata.cst8+0x60): undefined reference to `wf12'
/tmp/ccEfngi6.o:(.rodata.cst8+0x68): undefined reference to `wf12'
collect2: error: ld returned 1 exit status

FAIL: gcc.dg/attr-weakref-1.c (test for excess errors)

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Patch AArch64] Switch constant pools to separate rodata sections.
  2015-11-10 16:39     ` Alan Lawrence
@ 2015-11-10 16:47       ` Ramana Radhakrishnan
  2015-11-10 17:00       ` Alan Lawrence
  1 sibling, 0 replies; 11+ messages in thread
From: Ramana Radhakrishnan @ 2015-11-10 16:47 UTC (permalink / raw)
  To: Alan Lawrence; +Cc: Ramana Radhakrishnan, James Greenhalgh, gcc-patches

On Tue, Nov 10, 2015 at 4:39 PM, Alan Lawrence <alan.lawrence@arm.com> wrote:
> On 04/11/15 14:26, Ramana Radhakrishnan wrote:
>>
>>
>> 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.radhakrishnan@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.
>>
>
> Since r229878, I've been seeing
>
> FAIL: gcc.dg/attr-weakref-1.c (test for excess errors)
> UNRESOLVED: gcc.dg/attr-weakref-1.c compilation failed to produce executable
>
> (both previously passing) on aarch64-none-elf, aarch64_be-none-elf, and
> aarch64-none-linux-gnu. Here's a log from aarch64_be-none-elf (the others
> look similar):
>
> /work/alalaw01/build-aarch64_be-none-elf/obj/gcc2/gcc/xgcc
> -B/work/alalaw01/build-aarch64_be-none-elf/obj/gcc2/gcc/
> /work/alalaw01/src/gcc/gcc/testsuite/gcc.dg/attr-weakref-1.c
> -fno-diagnostics-show-caret -fdiagnostics-color=never -O2
> /work/alalaw01/src/gcc/gcc/testsuite/gcc.dg/attr-weakref-1a.c
> -specs=aem-validation.specs -lm -o ./attr-weakref-1.exe
> /tmp/ccEfngi6.o:(.rodata.cst8+0x30): undefined reference to `wv12'
> /tmp/ccEfngi6.o:(.rodata.cst8+0x38): undefined reference to `wv12'
> /tmp/ccEfngi6.o:(.rodata.cst8+0x60): undefined reference to `wf12'
> /tmp/ccEfngi6.o:(.rodata.cst8+0x68): undefined reference to `wf12'
> collect2: error: ld returned 1 exit status
> compiler exited with status 1
> output is:
> /tmp/ccEfngi6.o:(.rodata.cst8+0x30): undefined reference to `wv12'
> /tmp/ccEfngi6.o:(.rodata.cst8+0x38): undefined reference to `wv12'
> /tmp/ccEfngi6.o:(.rodata.cst8+0x60): undefined reference to `wf12'
> /tmp/ccEfngi6.o:(.rodata.cst8+0x68): undefined reference to `wf12'
> collect2: error: ld returned 1 exit status
>
> FAIL: gcc.dg/attr-weakref-1.c (test for excess errors)
>

Hmmm I'm surprised it failed in the first place as my testing didn't
show it - I need to check on that.

Nevertheless this fail has gone away in my testing with
https://gcc.gnu.org/ml/gcc-cvs/2015-11/msg00453.html in a bootstrap
and regression run on aarch64-none-linux-gnu. I see nothing triplet
specific in the testcase here for it to fail differently.

Is this something you see really with tip of trunk ?

regards
Ramana

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Patch AArch64] Switch constant pools to separate rodata sections.
  2015-11-10 16:39     ` Alan Lawrence
  2015-11-10 16:47       ` Ramana Radhakrishnan
@ 2015-11-10 17:00       ` Alan Lawrence
  1 sibling, 0 replies; 11+ messages in thread
From: Alan Lawrence @ 2015-11-10 17:00 UTC (permalink / raw)
  To: Ramana Radhakrishnan, James Greenhalgh; +Cc: gcc-patches

On 10/11/15 16:39, Alan Lawrence wrote:
> Since r229878, I've been seeing
>
> FAIL: gcc.dg/attr-weakref-1.c (test for excess errors)
> UNRESOLVED: gcc.dg/attr-weakref-1.c compilation failed to produce executable
>
> (both previously passing) on aarch64-none-elf, aarch64_be-none-elf, and
> aarch64-none-linux-gnu.

Ah, these are fixed by Ramana's partial rollback (r230085).

--Alan

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2015-11-10 17:00 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-11-03 16:36 [Patch AArch64] Switch constant pools to separate rodata sections Ramana Radhakrishnan
2015-11-03 17:10 ` James Greenhalgh
2015-11-04 14:26   ` Ramana Radhakrishnan
2015-11-05  9:35     ` James Greenhalgh
2015-11-08 11:43     ` Andreas Schwab
2015-11-09 14:24       ` Ramana Radhakrishnan
2015-11-09 16:46       ` Ramana Radhakrishnan
2015-11-09 17:21         ` James Greenhalgh
2015-11-10 16:39     ` Alan Lawrence
2015-11-10 16:47       ` Ramana Radhakrishnan
2015-11-10 17:00       ` Alan Lawrence

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).