public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Jan Hubicka <hubicka@ucw.cz>
To: Bernhard Reutner-Fischer <rep.dot.nop@gmail.com>
Cc: gcc-patches@gcc.gnu.org,
	Bernhard Reutner-Fischer <aldot@gcc.gnu.org>,
	gfortran ML <fortran@gcc.gnu.org>
Subject: Re: [PATCH 1/2] symtab: also change RTL decl name
Date: Tue, 22 Nov 2022 12:54:01 +0100	[thread overview]
Message-ID: <Y3y4WSs/I8YY5s5N@kam.mff.cuni.cz> (raw)
In-Reply-To: <20221121204743.6e4e7d57@nbbrfq>

> On Mon, 21 Nov 2022 20:02:49 +0100
> Jan Hubicka <hubicka@ucw.cz> wrote:
> 
> > > Hi Honza, Ping.
> > > Regtests cleanly for c,fortran,c++,ada,d,go,lto,objc,obj-c++
> > > Ok?
> > > I'd need this for attribute target_clones for the Fortran FE.  
> > Sorry for delay here.
> > > >  void
> > > > @@ -303,6 +301,10 @@ symbol_table::change_decl_assembler_name (tree decl, tree name)
> > > >  	warning (0, "%qD renamed after being referenced in assembly", decl);
> > > >  
> > > >        SET_DECL_ASSEMBLER_NAME (decl, name);
> > > > +      /* Set the new name in rtl.  */
> > > > +      if (DECL_RTL_SET_P (decl))
> > > > +	XSTR (XEXP (DECL_RTL (decl), 0), 0) = IDENTIFIER_POINTER (name);  
> > 
> > I am not quite sure how safe this is.  We generally produce DECL_RTL
> > when we produce assembly file.  So if DECL_RTL is set then we probably
> > already output the original function name and it is too late to change
> > it.
> 
> AFAICS we make_decl_rtl in the fortran FE in trans_function_start.

I see, it may be a relic of something that is no longer necessary.  Can
you see why one needs DECL_RTL so early?
> 
> > 
> > Also RTL is shared so changing it in-place is going to rewrite all the
> > existing RTL expressions using it.
> > 
> > Why the DECL_RTL is produced for function you want to rename?
> 
> I think the fortran FE sets it quite early when lowering a function.
> Later, when the ME creates the target_clones, it wants to rename the
> initial function to initial_fun.default for the default target.
> That's where the change_decl_assembler_name is called (only on the
> decl).
> But nobody changes the RTL name, so the ifunc (which should be the
> initial, unchanged name) is properly emitted but
> assemble_start_function uses the same, unchanged, initial fnname it
> later obtains by get_fnname_from_decl which fetches the (wrong) initial
> name where it should use the .default target name.
> See
> https://gcc.gnu.org/pipermail/gcc-patches/2022-November/605081.html
> 
> I'm open to other suggestions to make this work in a different way, of
> course. Maybe we're missing some magic somewhere that might share the
> name between the fndecl and the RTL XSTR so the RTL is magically
> updated by that single SET_ECL_ASSEMBLER_NAME in
> change_decl_assembler_name? But i didn't quite see where that'd be?

I think we should start by understanding why Fortran FE produces
DECL_RTL early.  It was written before symbol table code emerged, it may
be simply an oversight I made while converting FE to symbol table.

Honza
> 
> thanks,
> 
> > Honza
> > > > +
> > > >        if (alias)
> > > >  	{
> > > >  	  IDENTIFIER_TRANSPARENT_ALIAS (name) = 1;  
> > >   
> 

  reply	other threads:[~2022-11-22 11:54 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-11-09 19:02 [PATCH 0/2] Fortran: add attribute target_clones Bernhard Reutner-Fischer
2022-11-09 19:02 ` [PATCH 1/2] symtab: also change RTL decl name Bernhard Reutner-Fischer
2022-11-17  8:02   ` Bernhard Reutner-Fischer
2022-11-21 18:24     ` Mikael Morin
2022-11-21 19:02     ` Jan Hubicka
2022-11-21 19:47       ` Bernhard Reutner-Fischer
2022-11-22 11:54         ` Jan Hubicka [this message]
2023-02-19  2:29           ` Bernhard Reutner-Fischer
2022-11-09 19:02 ` [PATCH 2/2] Fortran: add attribute target_clones Bernhard Reutner-Fischer
2022-11-21 19:13   ` Mikael Morin
2022-11-21 22:26     ` Bernhard Reutner-Fischer
2022-11-22 13:17       ` Mikael Morin

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=Y3y4WSs/I8YY5s5N@kam.mff.cuni.cz \
    --to=hubicka@ucw.cz \
    --cc=aldot@gcc.gnu.org \
    --cc=fortran@gcc.gnu.org \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=rep.dot.nop@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).