Re: [PING] Re: [PATCH 2/2] Corrected pr25521.c target matching.

[Cupertino Miranda <cupertino.miranda@oracle.com>]

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