public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Jan Hubicka <hubicka@ucw.cz>
To: Michael Matz <matz@suse.de>
Cc: Jan Hubicka <hubicka@ucw.cz>, gcc-patches@gcc.gnu.org,	rguenther@suse.de
Subject: Re: Do not compute alias sets for types that don't need them
Date: Tue, 26 May 2015 19:00:00 -0000	[thread overview]
Message-ID: <20150526173450.GD43680@kam.mff.cuni.cz> (raw)
In-Reply-To: <alpine.LSU.2.20.1505261545450.27315@wotan.suse.de>

> Hi,
> 
> On Fri, 22 May 2015, Jan Hubicka wrote:
> 
> > Index: tree-streamer-out.c
> > ===================================================================
> > --- tree-streamer-out.c	(revision 223508)
> > +++ tree-streamer-out.c	(working copy)
> > @@ -346,6 +346,7 @@ pack_ts_type_common_value_fields (struct
> >       alias-set zero to this type.  */
> >    bp_pack_var_len_int (bp, (TYPE_ALIAS_SET (expr) == 0
> >  			    || (!in_lto_p
> > +				&& type_with_alias_set_p (expr)
> >  				&& get_alias_set (expr) == 0)) ? 0 : -1);
> 
> I find such interfaces very ugly.  IOW, when it's always (or often) 
> necessary to call check_foo_p() before foo() can be called then the 
> checking should be part of foo() (and it should then return a conservative 
> value, i.e. alias set 0), and that requirement not be imposed on the 
> callers of foo().  I.e. why can't whatever checks you do in 
> type_with_alias_set_p be included in get_alias_set?

Because of sanity checking: I want to make alias sets of those types undefined
rather than having random values.  The point is that using the alias set in
alias oracle querry is wrong.

Now I run into the case that we do produce MEM exprs for incomplete variants
just to take their address so I was thinking the other day about defining an
invalid alias set -2, making get_alias_set to return it and ICE later when query
is actually made?

We do have wrong query problems at least in ipa-icf, so I think it is worthwhile
sanity check.
> 
> > +     front-end routine) and use it.
> > +
> > +     We may be called to produce MEM RTX for variable of incomplete type.
> > +     This MEM RTX will only be used to produce address of a vairable, so
> > +     we do not need to compute alias set.  */
> > +  if (!DECL_P (t) || type_with_alias_set_p (TYPE_MAIN_VARIANT (TREE_TYPE (t))))
> > +    attrs.alias = get_alias_set (t);
> 
> And if the checking needs to go down the main-variant chain then this 
> should be done inside type_with_alias_set_p(), not in the caller, 
> otherwise even the symmetry between arguments of type_with_alias_set_p(xy) 
> and get_alias_set(xy) is destroyed (but see above for why I think 
> type_with_alias_set_p shouldn't even exist).

Yep, good point - I will cleanup this.

Honza

  reply	other threads:[~2015-05-26 17:34 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-05-22 12:33 Jan Hubicka
2015-05-22 13:32 ` Richard Biener
2015-05-22 13:42   ` Jan Hubicka
2015-05-26  8:16     ` Richard Biener
2015-05-27  6:33       ` Jan Hubicka
2015-05-27  7:59         ` Jan Hubicka
2015-05-27  8:50         ` Richard Biener
2015-05-26 14:07 ` Michael Matz
2015-05-26 19:00   ` Jan Hubicka [this message]
2015-05-27  8:39     ` Richard Biener
2015-05-27 16:24       ` Jan Hubicka

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=20150526173450.GD43680@kam.mff.cuni.cz \
    --to=hubicka@ucw.cz \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=matz@suse.de \
    --cc=rguenther@suse.de \
    /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).