Message ID | 20121024145004.GZ3263@bubble.grove.modra.org |
---|---|
State | New |
Headers | show |
Hello Alan, thanks for looking at this issue. On 10/24/2012 04:50 PM, Alan Modra wrote: > On Tue, Oct 23, 2012 at 06:25:43PM +0200, Sebastian Huber wrote: >> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55033 >> >> This patch fixes my problem, but I am absolutely not sure if this is the >> right way. > [snip] > > This is http://gcc.gnu.org/bugzilla/show_bug.cgi?id=9571 all over again. > > IMHO your patch is not wrong, but a better idea is that if we've used > categorize_decl_for_section to choose a section name, then we ought to > also use categorize_decl_for_section to choose section flags. Yes, I thought about this too, but I know to little about GCC to do this on my own. > > My original fix for pr9571 was to pass the decl down to > get_named_section. > http://gcc.gnu.org/ml/gcc-patches/2004-11/msg02487.html > That hit the assert in get_named_section when building libgfortran for > ia64, and my knee-jerk patch to fix that was to simply satisfy the > assert. After looking at all the target section_type_flags functions, > I believe the following is what I should have done way back then. > Bootstrapped and regression tested powerpc64-linux. > > * varasm.c (default_elf_select_section): Move !DECL_P check.. > (get_named_section): ..to here before calling get_section_name. > Adjust assertion. > (default_section_type_flags): Add DECL_P check. > * config/i386/winnt.c (i386_pe_section_type_flags): Likewise. > * config/rs6000/rs6000.c (rs6000_xcoff_section_type_flags): Likewise. Your change fixes the problem in PR55033. I can also run the RTEMS test suite with it, so from my point of view this looks perfect.
Hello, what needs to be done to get this committed?
Hello, is it possible to integrate this patch into the GCC 4.8 release? On 10/24/2012 04:50 PM, Alan Modra wrote: > On Tue, Oct 23, 2012 at 06:25:43PM +0200, Sebastian Huber wrote: >> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55033 >> >> This patch fixes my problem, but I am absolutely not sure if this is the >> right way. > [snip] > > This is http://gcc.gnu.org/bugzilla/show_bug.cgi?id=9571 all over again. > > IMHO your patch is not wrong, but a better idea is that if we've used > categorize_decl_for_section to choose a section name, then we ought to > also use categorize_decl_for_section to choose section flags. > > My original fix for pr9571 was to pass the decl down to > get_named_section. > http://gcc.gnu.org/ml/gcc-patches/2004-11/msg02487.html > That hit the assert in get_named_section when building libgfortran for > ia64, and my knee-jerk patch to fix that was to simply satisfy the > assert. After looking at all the target section_type_flags functions, > I believe the following is what I should have done way back then. > Bootstrapped and regression tested powerpc64-linux. > > * varasm.c (default_elf_select_section): Move !DECL_P check.. > (get_named_section): ..to here before calling get_section_name. > Adjust assertion. > (default_section_type_flags): Add DECL_P check. > * config/i386/winnt.c (i386_pe_section_type_flags): Likewise. > * config/rs6000/rs6000.c (rs6000_xcoff_section_type_flags): Likewise. > > Index: gcc/varasm.c > =================================================================== > --- gcc/varasm.c (revision 192660) > +++ gcc/varasm.c (working copy) > @@ -403,12 +403,16 @@ get_named_section (tree decl, const char *name, in > { > unsigned int flags; > > - gcc_assert (!decl || DECL_P (decl)); > if (name == NULL) > - name = TREE_STRING_POINTER (DECL_SECTION_NAME (decl)); > + { > + gcc_assert (decl && DECL_P (decl) && DECL_SECTION_NAME (decl)); > + name = TREE_STRING_POINTER (DECL_SECTION_NAME (decl)); > + } > > flags = targetm.section_type_flags (decl, name, reloc); > > + if (decl && !DECL_P (decl)) > + decl = NULL_TREE; > return get_section (name, flags, decl); > } > > @@ -5943,7 +5947,7 @@ default_section_type_flags (tree decl, const char > flags |= SECTION_RELRO; > } > > - if (decl && DECL_ONE_ONLY (decl)) > + if (decl && DECL_P (decl) && DECL_ONE_ONLY (decl)) > flags |= SECTION_LINKONCE; > > if (decl && TREE_CODE (decl) == VAR_DECL && DECL_THREAD_LOCAL_P (decl)) > @@ -6299,8 +6303,6 @@ default_elf_select_section (tree decl, int reloc, > gcc_unreachable (); > } > > - if (!DECL_P (decl)) > - decl = NULL_TREE; > return get_named_section (decl, sname, reloc); > } > > Index: gcc/config/i386/winnt.c > =================================================================== > --- gcc/config/i386/winnt.c (revision 192660) > +++ gcc/config/i386/winnt.c (working copy) > @@ -476,7 +476,7 @@ i386_pe_section_type_flags (tree decl, const char > flags |= SECTION_PE_SHARED; > } > > - if (decl && DECL_ONE_ONLY (decl)) > + if (decl && DECL_P (decl) && DECL_ONE_ONLY (decl)) > flags |= SECTION_LINKONCE; > > /* See if we already have an entry for this section. */ > Index: gcc/config/rs6000/rs6000.c > =================================================================== > --- gcc/config/rs6000/rs6000.c (revision 192660) > +++ gcc/config/rs6000/rs6000.c (working copy) > @@ -25689,7 +25689,7 @@ rs6000_xcoff_section_type_flags (tree decl, const > unsigned int flags = default_section_type_flags (decl, name, reloc); > > /* Align to at least UNIT size. */ > - if (flags & SECTION_CODE || !decl) > + if ((flags & SECTION_CODE) != 0 || !decl || !DECL_P (decl)) > align = MIN_UNITS_PER_WORD; > else > /* Increase alignment of large objects if not already stricter. */ >
On 01/29/2013 03:40 PM, Sebastian Huber wrote: > Hello, > > is it possible to integrate this patch into the GCC 4.8 release? With this patch we have: LAST_UPDATED: Mon Jan 28 07:50:05 UTC 2013 (revision 5c2b636:7d84cac:9fc8b0bfcfe944067fc4c75d8b3f97eab67cecf3) Native configuration is x86_64-unknown-linux-gnu === gcc tests === Running target unix XPASS: gcc.dg/inline_3.c (test for excess errors) XPASS: gcc.dg/inline_4.c (test for excess errors) XPASS: gcc.dg/unroll_2.c (test for excess errors) XPASS: gcc.dg/unroll_3.c (test for excess errors) XPASS: gcc.dg/unroll_4.c (test for excess errors) === gcc Summary === # of expected passes 92116 # of unexpected successes 5 # of expected failures 257 # of unsupported tests 1522 /scratch/git-build/b-gcc-git-linux/gcc/xgcc version 4.8.0 20130128 (experimental) [master revision 5c2b636:7d84cac:9fc8b0bfcfe944067fc4c75d8b3f97eab67cecf3] (GCC) === g++ tests === Running target unix === g++ Summary === # of expected passes 53540 # of expected failures 289 # of unsupported tests 738 /scratch/git-build/b-gcc-git-linux/gcc/testsuite/g++/../../xg++ version 4.8.0 20130128 (experimental) [master revision 5c2b636:7d84cac:9fc8b0bfcfe944067fc4c75d8b3f97eab67cecf3] (GCC) Compiler version: 4.8.0 20130128 (experimental) [master revision 5c2b636:7d84cac:9fc8b0bfcfe944067fc4c75d8b3f97eab67cecf3] (GCC) Platform: x86_64-unknown-linux-gnu configure flags: --prefix=/scratch/install-gcc-git --verbose --with-gnu-as --with-gnu-ld --enable-languages=c,c++ Without this patch we have: LAST_UPDATED: Mon Jan 28 07:50:05 UTC 2013 (revision 5c2b636:7d84cac:9fc8b0bfcfe944067fc4c75d8b3f97eab67cecf3) Native configuration is x86_64-unknown-linux-gnu === gcc tests === Running target unix XPASS: gcc.dg/inline_3.c (test for excess errors) XPASS: gcc.dg/inline_4.c (test for excess errors) XPASS: gcc.dg/unroll_2.c (test for excess errors) XPASS: gcc.dg/unroll_3.c (test for excess errors) XPASS: gcc.dg/unroll_4.c (test for excess errors) === gcc Summary === # of expected passes 92116 # of unexpected successes 5 # of expected failures 257 # of unsupported tests 1522 /scratch/git-build/b-gcc-git-linux/gcc/xgcc version 4.8.0 20130128 (experimental) [master revision 5c2b636:7d84cac:9fc8b0bfcfe944067fc4c75d8b3f97eab67cecf3] (GCC) === g++ tests === Running target unix === g++ Summary === # of expected passes 53540 # of expected failures 289 # of unsupported tests 738 /scratch/git-build/b-gcc-git-linux/gcc/testsuite/g++/../../xg++ version 4.8.0 20130128 (experimental) [master revision 5c2b636:7d84cac:9fc8b0bfcfe944067fc4c75d8b3f97eab67cecf3] (GCC) Compiler version: 4.8.0 20130128 (experimental) [master revision 5c2b636:7d84cac:9fc8b0bfcfe944067fc4c75d8b3f97eab67cecf3] (GCC) Platform: x86_64-unknown-linux-gnu configure flags: --prefix=/scratch/install-gcc-git --verbose --with-gnu-as --with-gnu-ld --enable-languages=c,c++ => No new regressions on platform x86_64-unknown-linux-gnu for C and C++ Some PowerPC results are here: http://gcc.gnu.org/ml/gcc-testresults/2013-01/msg03064.html
Index: gcc/varasm.c =================================================================== --- gcc/varasm.c (revision 192660) +++ gcc/varasm.c (working copy) @@ -403,12 +403,16 @@ get_named_section (tree decl, const char *name, in { unsigned int flags; - gcc_assert (!decl || DECL_P (decl)); if (name == NULL) - name = TREE_STRING_POINTER (DECL_SECTION_NAME (decl)); + { + gcc_assert (decl && DECL_P (decl) && DECL_SECTION_NAME (decl)); + name = TREE_STRING_POINTER (DECL_SECTION_NAME (decl)); + } flags = targetm.section_type_flags (decl, name, reloc); + if (decl && !DECL_P (decl)) + decl = NULL_TREE; return get_section (name, flags, decl); } @@ -5943,7 +5947,7 @@ default_section_type_flags (tree decl, const char flags |= SECTION_RELRO; } - if (decl && DECL_ONE_ONLY (decl)) + if (decl && DECL_P (decl) && DECL_ONE_ONLY (decl)) flags |= SECTION_LINKONCE; if (decl && TREE_CODE (decl) == VAR_DECL && DECL_THREAD_LOCAL_P (decl)) @@ -6299,8 +6303,6 @@ default_elf_select_section (tree decl, int reloc, gcc_unreachable (); } - if (!DECL_P (decl)) - decl = NULL_TREE; return get_named_section (decl, sname, reloc); } Index: gcc/config/i386/winnt.c =================================================================== --- gcc/config/i386/winnt.c (revision 192660) +++ gcc/config/i386/winnt.c (working copy) @@ -476,7 +476,7 @@ i386_pe_section_type_flags (tree decl, const char flags |= SECTION_PE_SHARED; } - if (decl && DECL_ONE_ONLY (decl)) + if (decl && DECL_P (decl) && DECL_ONE_ONLY (decl)) flags |= SECTION_LINKONCE; /* See if we already have an entry for this section. */ Index: gcc/config/rs6000/rs6000.c =================================================================== --- gcc/config/rs6000/rs6000.c (revision 192660) +++ gcc/config/rs6000/rs6000.c (working copy) @@ -25689,7 +25689,7 @@ rs6000_xcoff_section_type_flags (tree decl, const unsigned int flags = default_section_type_flags (decl, name, reloc); /* Align to at least UNIT size. */ - if (flags & SECTION_CODE || !decl) + if ((flags & SECTION_CODE) != 0 || !decl || !DECL_P (decl)) align = MIN_UNITS_PER_WORD; else /* Increase alignment of large objects if not already stricter. */