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))))
