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

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

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