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
