public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Jan Hubicka <hubicka@ucw.cz>
To: Richard Biener <rguenther@suse.de>
Cc: Jan Hubicka <hubicka@ucw.cz>, Michael Matz <matz@suse.de>,
	gcc-patches@gcc.gnu.org
Subject: Re: Do not compute alias sets for types that don't need them
Date: Wed, 27 May 2015 16:24:00 -0000	[thread overview]
Message-ID: <20150527161153.GE88897@kam.mff.cuni.cz> (raw)
In-Reply-To: <alpine.LSU.2.11.1505271026590.30088@zhemvz.fhfr.qr>

> On Tue, 26 May 2015, Jan Hubicka wrote:
> 
> > > 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.
> 
> You could have just returned 0 for the alias-set for 
> !type_with_alias_set_p in get_alias_set.  That avoids polluting the
> alias data structures and is neither random or wrong.

Take the example of bug in ipa-ICF. It is digging out completely random types
from the IL and thinks it absolutely must compare alias sets of all of them
(the bug obviously is that it really should compare only those that matters).
It then throws random incomplete type to get_alias_set and obtain 0.  Which will
make it to silently give up if the "matching" random type is complete.

ICE here is a friendly reminder to the author of the optimization pass that he
is doing something fishy. It will also catch the cases where we throw memory access
of incomplete type to the function body by frontend/middleend bug instead of just
silently disabling optimization. I caught the Java interface glue issue with this.
(still need to fix that)

Now pack_ts_type_common_value_fields and RTL generation are differnt from the usual use of
alias set oracle in a sense that they do compute unnecesary alias sets by design.
They are not optimizations, they are IL stage transitions.

Honza

      reply	other threads:[~2015-05-27 16:11 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
2015-05-27  8:39     ` Richard Biener
2015-05-27 16:24       ` Jan Hubicka [this message]

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=20150527161153.GE88897@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).