Cupertino Miranda via Gcc-patches writes:
> Thank you for the comments and suggestions. > I have changed the patch. > > Unfortunately in case of rx target I could not make > scan-assembler-symbol-section to match. I believe it is because the > .section and .global entries order is reversed in this target. > > Patch in inlined below. looking forward to your comments. > > Cupertino > > diff --git a/gcc/testsuite/gcc.dg/pr25521.c b/gcc/testsuite/gcc.dg/pr25521.c > index 63363a03b9f..82b4cd88ec0 100644 > --- a/gcc/testsuite/gcc.dg/pr25521.c > +++ b/gcc/testsuite/gcc.dg/pr25521.c > @@ -2,9 +2,10 @@ > sections. > > { dg-require-effective-target elf } > - { dg-do compile } */ > + { dg-do compile } > + { dg-skip-if "" { ! const_volatile_readonly_section } } */ > > const volatile int foo = 30; > > - > -/* { dg-final { scan-assembler "\\.s\?rodata" } } */ > +/* { dg-final { scan-assembler {.section C,} { target { rx-*-* } } } } */ > +/* { dg-final { scan-assembler-symbol-section {^_?foo$} > {^\.(const|s?rodata)} { target { ! "rx-*-*" } } } } */ > diff --git a/gcc/testsuite/lib/target-supports.exp > b/gcc/testsuite/lib/target-supports.exp > index c0694af2338..91aafd89909 100644 > --- a/gcc/testsuite/lib/target-supports.exp > +++ b/gcc/testsuite/lib/target-supports.exp > @@ -12295,3 +12295,13 @@ proc check_is_prog_name_available { prog } { > > return 1 > } > + > +# returns 1 if target does selects a readonly section for const volatile > variables. > +proc check_effective_target_const_volatile_readonly_section { } { > + > + if { [istarget powerpc-*-*] > + || [check-flags { "" { powerpc64-*-* } { -m32 } }] } { > + return 0 > + } > + return 1 > +} > > > Jeff Law writes: > >> On 12/7/22 08:45, Cupertino Miranda wrote: >>> >>>> On 12/2/22 10:52, Cupertino Miranda via Gcc-patches wrote: >>>>> This commit is a follow up of bugzilla #107181. >>>>> The commit /a0aafbc/ changed the default implementation of the >>>>> SELECT_SECTION hook in order to match clang/llvm behaviour w.r.t the >>>>> placement of `const volatile' objects. >>>>> However, the following targets use target-specific selection functions >>>>> and they choke on the testcase pr25521.c: >>>>> *rx - target sets its const variables as '.section C,"a",@progbits'. >>>> That's presumably a constant section. We should instead twiddle the test >>>> to >>>> recognize that section. >>> Although @progbits is indeed a constant section, I believe it is >>> more interesting to detect if the `rx' starts selecting more >>> standard sections instead of the current @progbits. >>> That was the reason why I opted to XFAIL instead of PASSing it. >>> Can I keep it as such ? >> I'm not aware of any ongoing development for that port, so I would not let >> concerns about the rx port changing behavior dominate how we approach this >> problem. >> >> The rx port is using a different name for the section. That's valid thing >> to >> do and to the extent we can, we should support that in the test rather than >> (incorrectly IMHO) xfailing the test just becuase the name isn't what we >> expected. >> >> To avoid over-eagerly matching, I would probably search for "C," I wouldn't >> do >> that for the const or rodata sections as they often have a suffix like 1, 2, >> 4, >> 8 for different sized rodata sections. >> >> PPC32 is explicitly doing something different and placing those objects into >> an >> RW section. So for PPC32 it makes more sense to skip the test rather than >> xfail >> it. >> >> Jeff