PING !!!!!
Cupertino Miranda via Gcc-patches writes: > 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