Hi Jeff,
Can you please confirm if the patch is Ok? Thanks, Cupertino > 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