public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Jeff Law <law@redhat.com>
To: Martin Sebor <msebor@gmail.com>,
	Richard Biener <richard.guenther@gmail.com>
Cc: gcc-patches <gcc-patches@gcc.gnu.org>
Subject: Re: [RFA] [tree-optimization/80576] Handle non-constant sizes in DSE
Date: Fri, 23 Aug 2019 16:50:00 -0000	[thread overview]
Message-ID: <1c854547-8fa8-197d-0200-c84336ec996b@redhat.com> (raw)
In-Reply-To: <c0b39b9b-092f-c38a-b1e1-e1c302635986@gmail.com>

On 8/22/19 8:40 AM, Martin Sebor wrote:
> On 8/21/19 4:47 PM, Jeff Law wrote:
>> On 8/19/19 7:57 AM, Richard Biener wrote:
>>
>>>
>>> +static tree
>>> +objsize_from_type (tree object)
>>> +{
>>> +  if (TREE_CODE (object) != ADDR_EXPR)
>>> +    return NULL_TREE;
>>> +
>>> +  tree type = TREE_TYPE (object);
>>> +  if (POINTER_TYPE_P (type))
>>>
>>> that if looks suspicious...  I'd say
>>>    if (!POINTER_TYPE_P (type))
>>>      return NULL_TREE;
>>>
>>> is better
>> Sure.  But we may not be able to use this anyway as you noted later,
>> depending on the type is not safe.
>>
>>>
>>> +    type = TREE_TYPE (type);
>>>
>>> +  if (TREE_CODE (type) == ARRAY_TYPE && !array_at_struct_end_p
>>> (object))
>>> +    {
>>>
>>> array_at_struct_end_p will never return true here since you pass it
>>> an ADDR_EXPR...  you wanted to pass TREE_OPERAND (object, 0) here?
>> I think you're right.  Given this was cribbed from the tail of another
>> function elsewhere, I suspect that other function has the same issue.
>>
>> I suspect ultimately we want to fix that other copy and drop
>> objsize_from_type.
> 
> Is the other copy compute_objsize in builtins.c?  The function
> has a comment that says it "is intended for diagnostics a should
> not be used to influence code generation or optimization."
> I added that comment on your request, not because the function
> relies the type of objects but because it conservatively uses
> the lower bound of ranges of offsets into arrays to determine
> the minimum size.
That's the one.  What's sad here is when I copied the tail of that
routine I noted the comment.  By the time I came back to the changes
several days (or weeks?) later the issue had been totally wiped from my
working memory.

I think I can just call get_base_address on the 0th operand of the
ADDR_EXPR and if I get back a _DECL node extract its DECL_UNIT_SIZE.

Jeff

  reply	other threads:[~2019-08-23 15:43 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-08-16 18:10 Jeff Law
2019-08-16 19:55 ` Marc Glisse
2019-08-16 20:41   ` Jeff Law
2019-08-16 22:49     ` Martin Sebor
2019-08-22  0:30       ` Jeff Law
2019-08-22 18:50         ` Martin Sebor
2019-08-23 16:50           ` Jeff Law
2019-08-19 14:23     ` Richard Biener
2019-08-22  2:12       ` Jeff Law
2019-08-22 11:14         ` Richard Biener
2019-08-23 20:27           ` Jeff Law
2019-08-26 10:07             ` Richard Biener
2019-09-03 21:24               ` Jeff Law
2019-09-09 20:10               ` Jeff Law
2019-09-16  9:12                 ` Richard Biener
2019-09-16  9:18                   ` Richard Biener
2019-08-22 15:53         ` Martin Sebor
2019-08-23 16:50           ` Jeff Law [this message]
2019-08-16 21:50   ` Jeff Law
2019-08-16 22:19     ` Marc Glisse
2019-08-16 22:43       ` 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=1c854547-8fa8-197d-0200-c84336ec996b@redhat.com \
    --to=law@redhat.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=msebor@gmail.com \
    --cc=richard.guenther@gmail.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).