On 03/15/2018 07:07 AM, Bin.Cheng wrote:
> On Fri, Feb 16, 2018 at 5:18 PM, <vladimir.mezent...@oracle.com> wrote:
>> From: Vladimir Mezentsev <vladimir.mezent...@oracle.com>
>>
>> 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 (!&Wv12);
chk (!&wv12);
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 <rguenth@138bc75d-0d04-0410-961f-82ee72b054a4>
> Date: Thu Mar 15 08:55:04 2018 +0000
>
> 2018-03-15 Vladimir Mezentsev <vladimir.mezent...@oracle.com>
>
> 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 <vladimir.mezent...@oracle.com>
>>
>> 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
>>
