[PING]
Cupertino Miranda writes:
> Hi Jeff,
>
> Please, please, give me some feedback on this one.
> I just don't want to have to keep asking you for time on this small
> pending patches that I also have to keep track on.
>
> I realized your committed the other one. Thank you !
>
> Best regards,
> Cupertino
>
>
> Cupertino Miranda writes:
>
>> 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
