| Message ID | 87furoy9gz.fsf@atmel.com |
|---|---|
| State | New |
| Headers | show |
Senthil Kumar Selvaraj writes: > Hi, > > This patch fixes a problem with fmerge-all-constants and the progmem > attribute - on trunk, the below testcase errors out with a section > conflict error. > > When avr_asm_select_section renames .rodata.xyz section to > .progmem.xyz and calls get_section, it passes in the same flags in > sect. If the flags include SECTION_DECLARED, get_section barfs with a > section conflict error - the section flag comparison logic strips off > SECTION_DECLARED from existing section flags before comparing it with > the new incoming flags. > > With -fmerge-all-constants, default_elf_select_section always returns > .rodata.strx.x. varasm switches to that section when writing out the > non progmem string literal, and that sets SECTION_DECLARED. The first > call to get_section with the section name transformed to > .progmem.data.strx.x then includes SECTION_DECLARED, but because this > is a new section, the section flag conflict logic doesn't kick in. The > second call to get_section, again including SECTION_DECLARED, triggers > the section flag conflict logic and causes the error. > > Stripping off SECTION_DECLARED before calling get_section fixes the > problem - the flag is supposed to be set by switch_section anyway. > > Reg testing showed no new regressions. Ok for trunk and backport to 6.x? > > Regards > Senthil > Added missing description in Changelog entry. gcc/testsuite/ChangeLog: 2016-07-05 Senthil Kumar Selvaraj <senthil_kumar.selvaraj@atmel.com> PR target/50739 * gcc.target/avr/pr50739.c: New test. gcc/ChangeLog: 2016-07-05 Senthil Kumar Selvaraj <senthil_kumar.selvaraj@atmel.com> PR target/50739 * config/avr/avr.c (avr_asm_select_section): Strip off SECTION_DECLARED from flags when calling get_section. Regards Senthil > > gcc/testsuite/ChangeLog: > > 2016-07-05 Senthil Kumar Selvaraj <senthil_kumar.selvaraj@atmel.com> > > PR target/50739 > * gcc.target/avr/pr50739.c: New test. > > > gcc/ChangeLog: > > 2016-07-05 Senthil Kumar Selvaraj <senthil_kumar.selvaraj@atmel.com> > > PR target/50739 > * config/avr/avr.c (avr_asm_select_section): > > > diff --git gcc/config/avr/avr.c gcc/config/avr/avr.c > index 18ed766..9b7b392 100644 > --- gcc/config/avr/avr.c > +++ gcc/config/avr/avr.c > @@ -9641,7 +9641,7 @@ avr_asm_select_section (tree decl, int reloc, unsigned HOST_WIDE_INT align) > { > const char *sname = ACONCAT ((new_prefix, > name + strlen (old_prefix), NULL)); > - return get_section (sname, sect->common.flags, sect->named.decl); > + return get_section (sname, sect->common.flags & ~SECTION_DECLARED, sect->named.decl); > } > } > > diff --git gcc/testsuite/gcc.target/avr/pr50739.c gcc/testsuite/gcc.target/avr/pr50739.c > new file mode 100644 > index 0000000..a6850b7 > --- /dev/null > +++ gcc/testsuite/gcc.target/avr/pr50739.c > @@ -0,0 +1,7 @@ > +/* { dg-do compile } */ > +/* { dg-options "-fmerge-all-constants" } */ > + > +char *ca = "123"; > + > +const char a[] __attribute__((__progmem__))= "a"; > +const char b[] __attribute__((__progmem__))= "b";
Senthil Kumar Selvaraj schrieb: > Senthil Kumar Selvaraj writes: > >> Hi, >> >> This patch fixes a problem with fmerge-all-constants and the progmem >> attribute - on trunk, the below testcase errors out with a section >> conflict error. >> >> When avr_asm_select_section renames .rodata.xyz section to >> .progmem.xyz and calls get_section, it passes in the same flags in >> sect. If the flags include SECTION_DECLARED, get_section barfs with a >> section conflict error - the section flag comparison logic strips off >> SECTION_DECLARED from existing section flags before comparing it with >> the new incoming flags. >> >> With -fmerge-all-constants, default_elf_select_section always returns >> .rodata.strx.x. varasm switches to that section when writing out the >> non progmem string literal, and that sets SECTION_DECLARED. The first >> call to get_section with the section name transformed to >> .progmem.data.strx.x then includes SECTION_DECLARED, but because this >> is a new section, the section flag conflict logic doesn't kick in. The >> second call to get_section, again including SECTION_DECLARED, triggers >> the section flag conflict logic and causes the error. >> >> Stripping off SECTION_DECLARED before calling get_section fixes the >> problem - the flag is supposed to be set by switch_section anyway. >> >> Reg testing showed no new regressions. Ok for trunk and backport to 6.x? >> >> Regards >> Senthil >> > > Added missing description in Changelog entry. > > gcc/testsuite/ChangeLog: > > 2016-07-05 Senthil Kumar Selvaraj <senthil_kumar.selvaraj@atmel.com> > > PR target/50739 > * gcc.target/avr/pr50739.c: New test. > > > gcc/ChangeLog: > > 2016-07-05 Senthil Kumar Selvaraj <senthil_kumar.selvaraj@atmel.com> > > PR target/50739 > * config/avr/avr.c (avr_asm_select_section): Strip off > SECTION_DECLARED from flags when calling get_section. > > Regards > Senthil > >> gcc/testsuite/ChangeLog: >> >> 2016-07-05 Senthil Kumar Selvaraj <senthil_kumar.selvaraj@atmel.com> >> >> PR target/50739 >> * gcc.target/avr/pr50739.c: New test. >> >> >> gcc/ChangeLog: >> >> 2016-07-05 Senthil Kumar Selvaraj <senthil_kumar.selvaraj@atmel.com> >> >> PR target/50739 >> * config/avr/avr.c (avr_asm_select_section): >> >> >> diff --git gcc/config/avr/avr.c gcc/config/avr/avr.c >> index 18ed766..9b7b392 100644 >> --- gcc/config/avr/avr.c >> +++ gcc/config/avr/avr.c >> @@ -9641,7 +9641,7 @@ avr_asm_select_section (tree decl, int reloc, unsigned HOST_WIDE_INT align) >> { >> const char *sname = ACONCAT ((new_prefix, >> name + strlen (old_prefix), NULL)); >> - return get_section (sname, sect->common.flags, sect->named.decl); >> + return get_section (sname, sect->common.flags & ~SECTION_DECLARED, sect->named.decl); Long line, should be wrapped. Johann >> } >> } >> >> diff --git gcc/testsuite/gcc.target/avr/pr50739.c gcc/testsuite/gcc.target/avr/pr50739.c >> new file mode 100644 >> index 0000000..a6850b7 >> --- /dev/null >> +++ gcc/testsuite/gcc.target/avr/pr50739.c >> @@ -0,0 +1,7 @@ >> +/* { dg-do compile } */ >> +/* { dg-options "-fmerge-all-constants" } */ >> + >> +char *ca = "123"; >> + >> +const char a[] __attribute__((__progmem__))= "a"; >> +const char b[] __attribute__((__progmem__))= "b"; > >
diff --git gcc/config/avr/avr.c gcc/config/avr/avr.c index 18ed766..9b7b392 100644 --- gcc/config/avr/avr.c +++ gcc/config/avr/avr.c @@ -9641,7 +9641,7 @@ avr_asm_select_section (tree decl, int reloc, unsigned HOST_WIDE_INT align) { const char *sname = ACONCAT ((new_prefix, name + strlen (old_prefix), NULL)); - return get_section (sname, sect->common.flags, sect->named.decl); + return get_section (sname, sect->common.flags & ~SECTION_DECLARED, sect->named.decl); } } diff --git gcc/testsuite/gcc.target/avr/pr50739.c gcc/testsuite/gcc.target/avr/pr50739.c new file mode 100644 index 0000000..a6850b7 --- /dev/null +++ gcc/testsuite/gcc.target/avr/pr50739.c @@ -0,0 +1,7 @@ +/* { dg-do compile } */ +/* { dg-options "-fmerge-all-constants" } */ + +char *ca = "123"; + +const char a[] __attribute__((__progmem__))= "a"; +const char b[] __attribute__((__progmem__))= "b";