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

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

[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