Re: [PATCH] PR gcc/68256 Defining TARGET_USE_CONSTANT_BLOCKS_P causes go bootstrap failure on aarch64.

[vladimir . mezentsev]

On 03/15/2018 07:07 AM, Bin.Cheng wrote:
> On Fri, Feb 16, 2018 at 5:18 PM,   wrote:
>> From: Vladimir Mezentsev 
>>
>> Ramana Radhakrishnan made a workaround in gcc/config/aarch64/aarch64.c to 
>> resolve
>> bootstrap comparison failure (2015-11-10, commit 
>> bc443a71dafa2e707bae4b2fa74f83b05dea37ab).
>> The real bug is in gcc/varasm.c.
>> hash_section() returns an unstable value.
>> As result, two blocks are created in get_block_for_section() for one unnamed 
>> section.
>> A list of objects in these blocks depends on the -gtoggle option.
>> I removed Ramana's workaround in gcc/config/aarch64/aarch64.c and  
>> I fixed hash_section() in gcc/varasm.c
>>
>> Bootstrapped on aarch64-unknown-linux-gnu including (c,c++ and go).
>> Testing finished ok.
> Hi Vladimir,
> Thanks for fixing the long standing issue, but this change causes below 
> failure,
> could you have a look?  Thanks
I did not see this failure.
I cannot tell why.
I use one CFARM machine (gcc116) and when my fix was approved I clean up
my directories
on this machine (including binaries and log files).

ramana.radhakrish...@arm.com implemented (2015-11-06, commit
cd4fcdb8ff16ec2dad56f9e736ac4552f8f52001)
a new feature ('Switch constant pools to separate rodata sections').
An additional fix is needed for this feature.

The test below demonstrates problem:
% cat t.c
extern void abort(void);
typedef int vtype;
static vtype Wv12 __attribute__((weakref ("wv12")));
extern vtype wv12 __attribute__((weak));

#define chk(p) do { if (!p) abort (); } while (0)

int main () {
  chk (!);
  chk (!);
  return (0);
}

% gcc  t.c
/tmp/cciZgRxK.o:(.rodata+0x0): undefined reference to `wv12'
/tmp/cciZgRxK.o:(.rodata+0x8): undefined reference to `wv12'

% gcc  t.c -S
% cat t.s

    .size   main, .-main
*    .weakref    Wv12,wv12   *<<   Not here. This should be
after definitions of Wv12 and wv12.
Wv12    .section    .rodata
    .align  3
.LC0:
    .xword  Wv12
    .align  3
.LC1:
    .xword  wv12


I can file a new PR and fix it by Tuesday/Wednesday
or we can  temporary restore Ramana's workaround in 
aarch64_use_blocks_for_constant_p:
% git diff
gcc/config/aarch64/aarch64.c
   

diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index 4b5183b..222ea33 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -7735,7 +7735,11 @@ 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::
+ return !aarch64_can_use_per_function_literal_pools_p ();
+ This return statement breaks gcc.dg/attr-weakref-1.c test.
+ For now we workaround this by returning false here.  */
+  return false;
 }
 
 /* Select appropriate section for constants depending


-Vladimir

>
> Failures:
> gcc.dg/attr-weakref-1.c
>
> Bisected to:
>
>
> commit 536728c16d6a0173930ecfe370302baa471c299e
> Author: rguenth 
> Date:   Thu Mar 15 08:55:04 2018 +
>
> 2018-03-15  Vladimir Mezentsev  
>
> PR target/68256
> * varasm.c (hash_section): Return an unchangeble hash value
> * config/aarch64/aarch64.c (aarch64_use_blocks_for_constant_p):
> Return !aarch64_can_use_per_function_literal_pools_p ().
>
>
> Thanks,
> bin
>> ChangeLog:
>> 2018-02-15  Vladimir Mezentsev  
>>
>> PR gcc/68256
>> * varasm.c (hash_section): Return an unchangeble hash value
>> * config/aarch64/aarch64.c (aarch64_use_blocks_for_constant_p):
>> Return !aarch64_can_use_per_function_literal_pools_p ();
>> ---
>>  gcc/config/aarch64/aarch64.c | 8 +++-
>>  gcc/varasm.c | 2 +-
>>  2 files changed, 4 insertions(+), 6 deletions(-)
>>
>> diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
>> index 174310c..a0a495d 100644
>> --- a/gcc/config/aarch64/aarch64.c
>> +++ b/gcc/config/aarch64/aarch64.c
>> @@ -7596,11 +7596,9 @@ aarch64_can_use_per_function_literal_pools_p (void)
>>  static bool
>>  aarch64_use_blocks_for_constant_p (machine_mode, const_rtx)
>>  {
>> -  /* 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;
>> +  /* 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 ();
>>  }
>>
>>  /* Select appropriate section for constants depending
>> diff --git a/gcc/varasm.c b/gcc/varasm.c
>> index b045efa..5aae5b4 100644
>> --- a/gcc/varasm.c
>> +++ 

Re: [PATCH] PR gcc/68256 Defining TARGET_USE_CONSTANT_BLOCKS_P causes go bootstrap failure on aarch64.

[Christophe Lyon]

On 15 March 2018 at 15:07, Bin.Cheng  wrote:
> On Fri, Feb 16, 2018 at 5:18 PM,   wrote:
>> From: Vladimir Mezentsev 
>>
>> Ramana Radhakrishnan made a workaround in gcc/config/aarch64/aarch64.c to 
>> resolve
>> bootstrap comparison failure (2015-11-10, commit 
>> bc443a71dafa2e707bae4b2fa74f83b05dea37ab).
>> The real bug is in gcc/varasm.c.
>> hash_section() returns an unstable value.
>> As result, two blocks are created in get_block_for_section() for one unnamed 
>> section.
>> A list of objects in these blocks depends on the -gtoggle option.
>> I removed Ramana's workaround in gcc/config/aarch64/aarch64.c and
>> I fixed hash_section() in gcc/varasm.c
>>
>> Bootstrapped on aarch64-unknown-linux-gnu including (c,c++ and go).
>> Testing finished ok.
> Hi Vladimir,
> Thanks for fixing the long standing issue, but this change causes below 
> failure,
> could you have a look?  Thanks
>
> Failures:
> gcc.dg/attr-weakref-1.c
>
> Bisected to:
>
>
> commit 536728c16d6a0173930ecfe370302baa471c299e
> Author: rguenth 
> Date:   Thu Mar 15 08:55:04 2018 +
>
> 2018-03-15  Vladimir Mezentsev  
>
> PR target/68256
> * varasm.c (hash_section): Return an unchangeble hash value
> * config/aarch64/aarch64.c (aarch64_use_blocks_for_constant_p):
> Return !aarch64_can_use_per_function_literal_pools_p ().
>

I see this too:
/aci-gcc-fsf/builds/gcc-fsf-gccsrc/obj-aarch64-none-linux-gnu/gcc3/gcc/xgcc
-B/aci-gcc-fsf/builds/gcc-fsf-gccsrc/obj-aarch64-none-linux-gnu/gcc3/gcc/
/gcc/testsuite/gcc.dg/attr-weakref-1.c -fno-diagnostics-show-caret
-fdiagnostics-color=never -O2 /gcc/testsuite/gcc.dg/attr-weakref-1a.c
-lm -o ./attr-weakref-1.exe
/ccLqgn8f.o:(.rodata.cst8+0x30): undefined reference to `wv12'
/ccLqgn8f.o:(.rodata.cst8+0x38): undefined reference to `wv12'
/ccLqgn8f.o:(.rodata.cst8+0x60): undefined reference to `wf12'
/ccLqgn8f.o:(.rodata.cst8+0x68): undefined reference to `wf12'
collect2: error: ld returned 1 exit status
compiler exited with status 1
FAIL: gcc.dg/attr-weakref-1.c (test for excess errors)

Christophe

>
> Thanks,
> bin
>>
>> ChangeLog:
>> 2018-02-15  Vladimir Mezentsev  
>>
>> PR gcc/68256
>> * varasm.c (hash_section): Return an unchangeble hash value
>> * config/aarch64/aarch64.c (aarch64_use_blocks_for_constant_p):
>> Return !aarch64_can_use_per_function_literal_pools_p ();
>> ---
>>  gcc/config/aarch64/aarch64.c | 8 +++-
>>  gcc/varasm.c | 2 +-
>>  2 files changed, 4 insertions(+), 6 deletions(-)
>>
>> diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
>> index 174310c..a0a495d 100644
>> --- a/gcc/config/aarch64/aarch64.c
>> +++ b/gcc/config/aarch64/aarch64.c
>> @@ -7596,11 +7596,9 @@ aarch64_can_use_per_function_literal_pools_p (void)
>>  static bool
>>  aarch64_use_blocks_for_constant_p (machine_mode, const_rtx)
>>  {
>> -  /* 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;
>> +  /* 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 ();
>>  }
>>
>>  /* Select appropriate section for constants depending
>> diff --git a/gcc/varasm.c b/gcc/varasm.c
>> index b045efa..5aae5b4 100644
>> --- a/gcc/varasm.c
>> +++ b/gcc/varasm.c
>> @@ -225,7 +225,7 @@ hash_section (section *sect)
>>  {
>>if (sect->common.flags & SECTION_NAMED)
>>  return htab_hash_string (sect->named.name);
>> -  return sect->common.flags;
>> +  return sect->common.flags & ~SECTION_DECLARED;
>>  }
>>
>>  /* Helper routines for maintaining object_block_htab.  */
>> --
>> 1.8.3.1
>>

Re: [PATCH] PR gcc/68256 Defining TARGET_USE_CONSTANT_BLOCKS_P causes go bootstrap failure on aarch64.

[Bin.Cheng]

On Fri, Feb 16, 2018 at 5:18 PM,   wrote:
> From: Vladimir Mezentsev 
>
> Ramana Radhakrishnan made a workaround in gcc/config/aarch64/aarch64.c to 
> resolve
> bootstrap comparison failure (2015-11-10, commit 
> bc443a71dafa2e707bae4b2fa74f83b05dea37ab).
> The real bug is in gcc/varasm.c.
> hash_section() returns an unstable value.
> As result, two blocks are created in get_block_for_section() for one unnamed 
> section.
> A list of objects in these blocks depends on the -gtoggle option.
> I removed Ramana's workaround in gcc/config/aarch64/aarch64.c and
> I fixed hash_section() in gcc/varasm.c
>
> Bootstrapped on aarch64-unknown-linux-gnu including (c,c++ and go).
> Testing finished ok.
Hi Vladimir,
Thanks for fixing the long standing issue, but this change causes below failure,
could you have a look?  Thanks

Failures:
gcc.dg/attr-weakref-1.c

Bisected to:


commit 536728c16d6a0173930ecfe370302baa471c299e
Author: rguenth 
Date:   Thu Mar 15 08:55:04 2018 +

2018-03-15  Vladimir Mezentsev  

PR target/68256
* varasm.c (hash_section): Return an unchangeble hash value
* config/aarch64/aarch64.c (aarch64_use_blocks_for_constant_p):
Return !aarch64_can_use_per_function_literal_pools_p ().


Thanks,
bin
>
> ChangeLog:
> 2018-02-15  Vladimir Mezentsev  
>
> PR gcc/68256
> * varasm.c (hash_section): Return an unchangeble hash value
> * config/aarch64/aarch64.c (aarch64_use_blocks_for_constant_p):
> Return !aarch64_can_use_per_function_literal_pools_p ();
> ---
>  gcc/config/aarch64/aarch64.c | 8 +++-
>  gcc/varasm.c | 2 +-
>  2 files changed, 4 insertions(+), 6 deletions(-)
>
> diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
> index 174310c..a0a495d 100644
> --- a/gcc/config/aarch64/aarch64.c
> +++ b/gcc/config/aarch64/aarch64.c
> @@ -7596,11 +7596,9 @@ aarch64_can_use_per_function_literal_pools_p (void)
>  static bool
>  aarch64_use_blocks_for_constant_p (machine_mode, const_rtx)
>  {
> -  /* 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;
> +  /* 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 ();
>  }
>
>  /* Select appropriate section for constants depending
> diff --git a/gcc/varasm.c b/gcc/varasm.c
> index b045efa..5aae5b4 100644
> --- a/gcc/varasm.c
> +++ b/gcc/varasm.c
> @@ -225,7 +225,7 @@ hash_section (section *sect)
>  {
>if (sect->common.flags & SECTION_NAMED)
>  return htab_hash_string (sect->named.name);
> -  return sect->common.flags;
> +  return sect->common.flags & ~SECTION_DECLARED;
>  }
>
>  /* Helper routines for maintaining object_block_htab.  */
> --
> 1.8.3.1
>

Re: [PATCH] PR gcc/68256 Defining TARGET_USE_CONSTANT_BLOCKS_P causes go bootstrap failure on aarch64.

[Richard Biener]

On Mon, Feb 26, 2018 at 12:07 PM, Richard Biener
 wrote:
> On Fri, Feb 16, 2018 at 6:18 PM,   wrote:
>> From: Vladimir Mezentsev 
>>
>> Ramana Radhakrishnan made a workaround in gcc/config/aarch64/aarch64.c to 
>> resolve
>> bootstrap comparison failure (2015-11-10, commit 
>> bc443a71dafa2e707bae4b2fa74f83b05dea37ab).
>> The real bug is in gcc/varasm.c.
>> hash_section() returns an unstable value.
>> As result, two blocks are created in get_block_for_section() for one unnamed 
>> section.
>> A list of objects in these blocks depends on the -gtoggle option.
>> I removed Ramana's workaround in gcc/config/aarch64/aarch64.c and
>> I fixed hash_section() in gcc/varasm.c
>>
>> Bootstrapped on aarch64-unknown-linux-gnu including (c,c++ and go).
>> Testing finished ok.
>
> Ok.

Committed on behalf of Vladimir.  r258553.

Richard.

> Thanks,
> Richard.
>
>> ChangeLog:
>> 2018-02-15  Vladimir Mezentsev  
>>
>> PR gcc/68256
>> * varasm.c (hash_section): Return an unchangeble hash value
>> * config/aarch64/aarch64.c (aarch64_use_blocks_for_constant_p):
>> Return !aarch64_can_use_per_function_literal_pools_p ();
>> ---
>>  gcc/config/aarch64/aarch64.c | 8 +++-
>>  gcc/varasm.c | 2 +-
>>  2 files changed, 4 insertions(+), 6 deletions(-)
>>
>> diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
>> index 174310c..a0a495d 100644
>> --- a/gcc/config/aarch64/aarch64.c
>> +++ b/gcc/config/aarch64/aarch64.c
>> @@ -7596,11 +7596,9 @@ aarch64_can_use_per_function_literal_pools_p (void)
>>  static bool
>>  aarch64_use_blocks_for_constant_p (machine_mode, const_rtx)
>>  {
>> -  /* 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;
>> +  /* 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 ();
>>  }
>>
>>  /* Select appropriate section for constants depending
>> diff --git a/gcc/varasm.c b/gcc/varasm.c
>> index b045efa..5aae5b4 100644
>> --- a/gcc/varasm.c
>> +++ b/gcc/varasm.c
>> @@ -225,7 +225,7 @@ hash_section (section *sect)
>>  {
>>if (sect->common.flags & SECTION_NAMED)
>>  return htab_hash_string (sect->named.name);
>> -  return sect->common.flags;
>> +  return sect->common.flags & ~SECTION_DECLARED;
>>  }
>>
>>  /* Helper routines for maintaining object_block_htab.  */
>> --
>> 1.8.3.1
>>

Re: [PATCH] PR gcc/68256 Defining TARGET_USE_CONSTANT_BLOCKS_P causes go bootstrap failure on aarch64.

[Richard Biener]

On Fri, Feb 16, 2018 at 6:18 PM,   wrote:
> From: Vladimir Mezentsev 
>
> Ramana Radhakrishnan made a workaround in gcc/config/aarch64/aarch64.c to 
> resolve
> bootstrap comparison failure (2015-11-10, commit 
> bc443a71dafa2e707bae4b2fa74f83b05dea37ab).
> The real bug is in gcc/varasm.c.
> hash_section() returns an unstable value.
> As result, two blocks are created in get_block_for_section() for one unnamed 
> section.
> A list of objects in these blocks depends on the -gtoggle option.
> I removed Ramana's workaround in gcc/config/aarch64/aarch64.c and
> I fixed hash_section() in gcc/varasm.c
>
> Bootstrapped on aarch64-unknown-linux-gnu including (c,c++ and go).
> Testing finished ok.

Ok.

Thanks,
Richard.

> ChangeLog:
> 2018-02-15  Vladimir Mezentsev  
>
> PR gcc/68256
> * varasm.c (hash_section): Return an unchangeble hash value
> * config/aarch64/aarch64.c (aarch64_use_blocks_for_constant_p):
> Return !aarch64_can_use_per_function_literal_pools_p ();
> ---
>  gcc/config/aarch64/aarch64.c | 8 +++-
>  gcc/varasm.c | 2 +-
>  2 files changed, 4 insertions(+), 6 deletions(-)
>
> diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
> index 174310c..a0a495d 100644
> --- a/gcc/config/aarch64/aarch64.c
> +++ b/gcc/config/aarch64/aarch64.c
> @@ -7596,11 +7596,9 @@ aarch64_can_use_per_function_literal_pools_p (void)
>  static bool
>  aarch64_use_blocks_for_constant_p (machine_mode, const_rtx)
>  {
> -  /* 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;
> +  /* 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 ();
>  }
>
>  /* Select appropriate section for constants depending
> diff --git a/gcc/varasm.c b/gcc/varasm.c
> index b045efa..5aae5b4 100644
> --- a/gcc/varasm.c
> +++ b/gcc/varasm.c
> @@ -225,7 +225,7 @@ hash_section (section *sect)
>  {
>if (sect->common.flags & SECTION_NAMED)
>  return htab_hash_string (sect->named.name);
> -  return sect->common.flags;
> +  return sect->common.flags & ~SECTION_DECLARED;
>  }
>
>  /* Helper routines for maintaining object_block_htab.  */
> --
> 1.8.3.1
>

[PATCH] PR gcc/68256 Defining TARGET_USE_CONSTANT_BLOCKS_P causes go bootstrap failure on aarch64.

[vladimir . mezentsev]

From: Vladimir Mezentsev 

Ramana Radhakrishnan made a workaround in gcc/config/aarch64/aarch64.c to 
resolve
bootstrap comparison failure (2015-11-10, commit 
bc443a71dafa2e707bae4b2fa74f83b05dea37ab).
The real bug is in gcc/varasm.c.
hash_section() returns an unstable value.
As result, two blocks are created in get_block_for_section() for one unnamed 
section.
A list of objects in these blocks depends on the -gtoggle option.
I removed Ramana's workaround in gcc/config/aarch64/aarch64.c and
I fixed hash_section() in gcc/varasm.c

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

ChangeLog:
2018-02-15  Vladimir Mezentsev  

PR gcc/68256
* varasm.c (hash_section): Return an unchangeble hash value
* config/aarch64/aarch64.c (aarch64_use_blocks_for_constant_p):
Return !aarch64_can_use_per_function_literal_pools_p ();
---
 gcc/config/aarch64/aarch64.c | 8 +++-
 gcc/varasm.c | 2 +-
 2 files changed, 4 insertions(+), 6 deletions(-)

diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index 174310c..a0a495d 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -7596,11 +7596,9 @@ aarch64_can_use_per_function_literal_pools_p (void)
 static bool
 aarch64_use_blocks_for_constant_p (machine_mode, const_rtx)
 {
-  /* 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;
+  /* 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 ();
 }
 
 /* Select appropriate section for constants depending
diff --git a/gcc/varasm.c b/gcc/varasm.c
index b045efa..5aae5b4 100644
--- a/gcc/varasm.c
+++ b/gcc/varasm.c
@@ -225,7 +225,7 @@ hash_section (section *sect)
 {
   if (sect->common.flags & SECTION_NAMED)
 return htab_hash_string (sect->named.name);
-  return sect->common.flags;
+  return sect->common.flags & ~SECTION_DECLARED;
 }
 
 /* Helper routines for maintaining object_block_htab.  */
-- 
1.8.3.1