On Sat, Mar 30, 2013 at 06:29:36PM -0400, David Edelsohn wrote: > How can we make progress to get this patch committed on trunk, 4.8 and 4.7?
I have OKs for the config/i386/winnt.c and config/rs6000/rs6000.c parts. I just need someone who is authorized to review patches to varasm.c, and is willing to risk their reputation to at least comment on the patch. Even a loud "NO" would be better than silence. As it stands, I'm sorry I offered the patch, even though I still believe the patch is correct from a design viewpoint, better than Sebastian's patch that just tackled the specific case of ".sdata2". His patch is obviously easier to review, we'd be OK on powerpc.. Let's try again with all the information in one place, and perhaps a better explanation. http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55033 is about a problem on powerpc eabi, where gcc generates wrong section flags (SECTION_WRITE) for an array initializer that should live in .sdata2, a read-only section, and then hits an internal consistency check because gcc gets the correct flags for .sdata2 in other cases. See attached C testcase. Sebastian offered a patch to default_section_type_flags that would correct the flags, but his approach is going back to the mess we had before Richard Henderson gave us categorize_decl_for_section. I made the comment that we ought to use categorize_decl_for_section for selecting the section flags, if we've used categorize_decl_for_section to select the section name. Specifically, default_elf_select_section should allow this to happen. rth agreed with this design approach in http://gcc.gnu.org/ml/gcc-patches/2004-11/msg02487.html, but that particular patch of mine was flawed, and I fixed a followup PR in a non-ideal way. So http://gcc.gnu.org/ml/gcc-patches/2012-10/msg02172.html is actually a fix for a very old patch of mine. I can well understand a reviewer looking at the patch and scratching their heads a little. Not so much due to the latest patch, but because existing code in this area is suspicious. For instance, you might wonder why it is correct to have if (decl && !DECL_P (decl)) decl = NULL_TREE; before calling get_section(). The answer is that get_section() is not prepared to handle !DECL_P trees when reporting errors. Arguably it should be modified to do that. -- Alan Modra Australia Development Lab, IBM
/* { dg-do compile { target powerpc*-*-eabi* powerpc*-*-elf* powerpc*-*-linux* } } */ /* { dg-require-effective-target ilp32 } */ /* { dg-options "-mcpu=8540 -msoft-float -meabi -msdata" } */ void f(void); struct s { int *p; int *q; }; extern int a; extern const struct s c; const struct s c = { &a, 0 }; void f(void) { char buf[4] = { 0, 1, 2, 3 }; }