Hi Jeff,
Kindly calling your attention to this thread.
Regards,
Cupertino
Cupertino Miranda via Gcc-patches writes:
> Richard Biener writes:
>
>> On Mon, Dec 5, 2022 at 7:07 PM Jeff Law via Gcc-patches
>> <gcc-patches@gcc.gnu.org> wrote:
>>>
>>>
>>>
>>> On 12/2/22 10:52, Cupertino Miranda via Gcc-patches wrote:
>>> > Changed target code to select .rodata section for 'const volatile'
>>> > defined variables.
>>> > This change is in the context of the bugzilla #170181.
>>> >
>>> > gcc/ChangeLog:
>>> >
>>> > v850.c(v850_select_section): Changed function.
>>> I'm not sure this is safe/correct. ISTM that you need to look at the
>>> underlying TREE_TYPE to check for const-volatile rather than
>>> TREE_SIDE_EFFECTS.
>>
>> Just to quote tree.h:
>>
>> /* In any expression, decl, or constant, nonzero means it has side effects or
>> reevaluation of the whole expression could produce a different value.
>> This is set if any subexpression is a function call, a side effect or a
>> reference to a volatile variable. In a ..._DECL, this is set only if the
>> declaration said `volatile'. This will never be set for a constant. */
>> #define TREE_SIDE_EFFECTS(NODE) \
>> (NON_TYPE_CHECK (NODE)->base.side_effects_flag)
>>
>> so if exp is a decl then that's the volatile check.
>>
>
> Thank you Richard for the review.
> Jeff: Can you please let me know if Richard comments reply to your
> concerns?
>
> Cupertino
>
>>> Of secondary importance is the ChangeLog. Just saying "Changed
>>> function" provides no real information. Something like this would be
>>> better:
>>>
>>> * config/v850/v850.c (v850_select_section): Put const volatile
>>> objects into read-only sections.
>>>
>>>
>>> Jeff
>>>
>>>
>>>
>>>
>>> > ---
>>> > gcc/config/v850/v850.cc | 1 -
>>> > 1 file changed, 1 deletion(-)
>>> >
>>> > diff --git a/gcc/config/v850/v850.cc b/gcc/config/v850/v850.cc
>>> > index c7d432990ab..e66893fede4 100644
>>> > --- a/gcc/config/v850/v850.cc
>>> > +++ b/gcc/config/v850/v850.cc
>>> > @@ -2865,7 +2865,6 @@ v850_select_section (tree exp,
>>> > {
>>> > int is_const;
>>> > if (!TREE_READONLY (exp)
>>> > - || TREE_SIDE_EFFECTS (exp)
>>> > || !DECL_INITIAL (exp)
>>> > || (DECL_INITIAL (exp) != error_mark_node
>>> > && !TREE_CONSTANT (DECL_INITIAL (exp))))
