Cupertino Miranda via Gcc-patches writes:
> gentle ping > > Cupertino Miranda writes: > >> Hi Jeff, >> >> First of all thanks for your quick review. >> Apologies for the delay replying, the message got lost in my inbox. >> >>> 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. >> >> I believe this was asked by Jose when he first sent the generic patches. >> Please notice my change is influenced by his original patch that does >> the same and was approved. >> >> https://gcc.gnu.org/pipermail/gcc-patches/2022-August/599348.html >> https://gcc.gnu.org/pipermail/gcc-patches/2022-September/602374.html >> >>> >>> 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))))
