public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Cupertino Miranda <cupertino.miranda@oracle.com>
To: Jeff Law <jeffreyalaw@gmail.com>
Cc: gcc-patches@gcc.gnu.org, jose.marchesi@oracle.com
Subject: Re: [PATCH 1/2] select .rodata for const volatile variables.
Date: Wed, 07 Dec 2022 15:12:07 +0000	[thread overview]
Message-ID: <878rjj5jug.fsf@oracle.com> (raw)
In-Reply-To: <b54fb524-336e-44b7-2092-abe9f82258fc@gmail.com>


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

  reply	other threads:[~2022-12-07 15:12 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-12-02 17:52 [PATCH] `const volatile' sections selection - bugzilla #107181 Cupertino Miranda
2022-12-02 17:52 ` [PATCH 1/2] select .rodata for const volatile variables Cupertino Miranda
2022-12-05 18:06   ` Jeff Law
2022-12-07 15:12     ` Cupertino Miranda [this message]
2022-12-15 10:13       ` Cupertino Miranda
2022-12-22 17:21         ` [PING] " Cupertino Miranda
2023-01-02 10:42           ` Cupertino Miranda
2023-01-09  7:57     ` Richard Biener
2023-01-13 15:06       ` Cupertino Miranda
2023-01-19  9:59         ` Cupertino Miranda
2023-01-22 18:43           ` Jeff Law
2023-01-22 18:49       ` Jeff Law
2022-12-02 17:52 ` [PATCH 2/2] Corrected pr25521.c target matching Cupertino Miranda
2022-12-05 18:47   ` Jeff Law
2022-12-07 15:45     ` Cupertino Miranda
2022-12-15 10:14       ` Cupertino Miranda
2022-12-22 17:22         ` [PING] " Cupertino Miranda
2023-01-02 10:43           ` Cupertino Miranda
2023-01-13 15:13       ` Cupertino Miranda
2023-01-22 19:04       ` Jeff Law
2023-01-24 12:24         ` Cupertino Miranda
2023-01-31  9:10           ` [PING] " Cupertino Miranda
2023-02-07  9:53             ` Cupertino Miranda
2023-02-17 14:33               ` Cupertino Miranda
2023-02-27 10:17                 ` Cupertino Miranda
2023-03-09  9:51                   ` [PING, PING] " Cupertino Miranda
2023-03-11 16:25           ` Jeff Law
2023-03-13 17:52             ` Cupertino Miranda
2023-03-13 17:57               ` Cupertino Miranda
2023-04-03  4:16                 ` Jeff Law

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=878rjj5jug.fsf@oracle.com \
    --to=cupertino.miranda@oracle.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=jeffreyalaw@gmail.com \
    --cc=jose.marchesi@oracle.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).