public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Jan Hubicka <hubicka@ucw.cz>
To: Richard Guenther <rguenther@suse.de>
Cc: Jan Hubicka <hubicka@ucw.cz>, "H.J. Lu" <hjl.tools@gmail.com>,
	gcc-patches@gcc.gnu.org, dnovillo@google.com, jakub@redhat.com
Subject: Re: varpool alias reorg
Date: Mon, 27 Jun 2011 16:13:00 -0000	[thread overview]
Message-ID: <20110627154815.GB23824@kam.mff.cuni.cz> (raw)
In-Reply-To: <alpine.LNX.2.00.1106271539010.810@zhemvz.fhfr.qr>

> On Fri, 24 Jun 2011, Jan Hubicka wrote:
> 
> > Hi,
> > this is yet another variant of the fix.  This time we stream builtins decls as
> > usually, but at fixup time we copy the assembler names (if set) into the
> > builtin decls used by folders.  Not sure if it is any better than breaking
> > memops-asm, but I can imagine that things like glibc actually rename string
> > functions into their internal variants (and thus with this version of patch we
> > would be able to LTO such library, but still we won't be able to LTO such
> > library into something else because something else would end up referncing the
> > internal versions of builtins).  I doubt we could do any better, however.
> 
> Not stream builtins with adjusted assembler names (I guess we'd need
> a flag for this, DECL_USER_ASSEMBLER_NAME_SET_P?  Or just check for

Most of code just checks for '*' on begginign of assembler name. I suppose it is safe.

> attributes?) as builtins but as new decls.  Let lto symbol merging
> then register those as aliases.  But which way around?  probably
> similar to how we should handle re-defined extern inlines, the
> extern inline being the GCC builtin and the re-definition being
> the aliased one.

I don't quite get your answer here.  What we do now is:

 1) stream in builtin as special kind of reference with decl assembler name associated to it
 2) at streaming in time always resolve builtlin to the "official" builtin decls (no matter
 what types and other stuff builtin had at stream out time) and overwritting the official builtin
 assembler name into one specified.

What i suggest is

 1) Stream out builtins as usual decls just with the extra function code
 2) Stream in builtins as usually
 3) optionally set the assembler name of the "official" decl

I see there are problems with i.e. one decl rule, but we do have same problems
with normal frontends that also do use different decl for explicit builtin
calls than for implicit, sadly.

I am not quite sure what the proper fix for this problem is - it is very handy
to have builtin decl in middle end where I know it is sane (i.e. it has the
right types etc.). Since C allows to declare the builtins arbitrarily, it gets
bit tricky to preserve one decl rule here.
> 
> > __attribute__ ((used)) is still needed in memops-asm-lib.c because LTO symtab
> > of course doesn't see the future references to builtins that we will emit
> > later via folding.  I think it is resonable requirement, as discussed at the
> > time enabling the plugin.
> 
> Yes, I think the testcase fix sounds reasonable.
> 
> I suppose you can come up with a simpler testcase for this "feature"
> for gcc.dg/lto highlighting the different issues?  I'm not sure
> if we are talking about my_memcpy () alias("memcpy") or
> memcpy () alias("my_memcpy").
> 
> I still like to stream unmodified builtins as builtins, as that is
> similar to pre-loading the streamer caches with things like
> void_type_node or sizetype.

Doing so will need us to solve the other one decl rules probly.
I didn't really got what the preloading is useful for after all?

Honza

> 
> Richard.

  reply	other threads:[~2011-06-27 15:48 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-06-18  9:11 Jan Hubicka
2011-06-18 14:49 ` H.J. Lu
2011-06-23 14:34   ` H.J. Lu
2011-06-23 16:44     ` Jan Hubicka
2011-06-24 12:30       ` Jan Hubicka
2011-06-24 12:33         ` Jan Hubicka
2011-06-27 14:08           ` Richard Guenther
2011-06-27 16:13             ` Jan Hubicka [this message]
2011-06-27 16:22               ` Michael Matz
2011-06-28  8:34               ` Richard Guenther
2011-06-27  9:43       ` Richard Guenther
2011-06-27  9:50         ` Jan Hubicka
2011-06-22  8:17 ` Regression with "varpool alias reorg" Hans-Peter Nilsson

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=20110627154815.GB23824@kam.mff.cuni.cz \
    --to=hubicka@ucw.cz \
    --cc=dnovillo@google.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=hjl.tools@gmail.com \
    --cc=jakub@redhat.com \
    --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).