| Message ID | 1518801506-27652-1-git-send-email-vladimir.mezentsev@oracle.com |
|---|---|
| State | New |
| Headers | show |
| Series | PR gcc/68256 Defining TARGET_USE_CONSTANT_BLOCKS_P causes go bootstrap failure on aarch64. | expand |
On Fri, Feb 16, 2018 at 6:18 PM, <vladimir.mezentsev@oracle.com> wrote: > From: Vladimir Mezentsev <vladimir.mezentsev@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. Ok. Thanks, Richard. > ChangeLog: > 2018-02-15 Vladimir Mezentsev <vladimir.mezentsev@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 >
On Mon, Feb 26, 2018 at 12:07 PM, Richard Biener <richard.guenther@gmail.com> wrote: > On Fri, Feb 16, 2018 at 6:18 PM, <vladimir.mezentsev@oracle.com> wrote: >> From: Vladimir Mezentsev <vladimir.mezentsev@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. > > Ok. Committed on behalf of Vladimir. r258553. Richard. > Thanks, > Richard. > >> ChangeLog: >> 2018-02-15 Vladimir Mezentsev <vladimir.mezentsev@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 >>
On Fri, Feb 16, 2018 at 5:18 PM, <vladimir.mezentsev@oracle.com> wrote: > From: Vladimir Mezentsev <vladimir.mezentsev@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 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.mezentsev@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.mezentsev@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 >
On 15 March 2018 at 15:07, Bin.Cheng <amker.cheng@gmail.com> wrote: > On Fri, Feb 16, 2018 at 5:18 PM, <vladimir.mezentsev@oracle.com> wrote: >> From: Vladimir Mezentsev <vladimir.mezentsev@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 > > 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.mezentsev@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 (). > 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 <vladimir.mezentsev@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 >>
On 03/15/2018 07:07 AM, Bin.Cheng wrote: > On Fri, Feb 16, 2018 at 5:18 PM, <vladimir.mezentsev@oracle.com> wrote: >> From: Vladimir Mezentsev <vladimir.mezentsev@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.radhakrishnan@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.mezentsev@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.mezentsev@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 >>
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. */
From: Vladimir Mezentsev <vladimir.mezentsev@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. ChangeLog: 2018-02-15 Vladimir Mezentsev <vladimir.mezentsev@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(-)