Re: IMA: backward-compatible behavior from lhd_set_decl_assembler_name

[Geoff Keating <geoffk at geoffk dot org>]

On 05/07/2004, at 11:41 PM, Zack Weinberg wrote:

> Geoff Keating <geoffk@geoffk.org> writes:
> [...]
> > This is expected to compile and not abort(), since it is strictly
> > conforming C (if you fix the closing brace).  According to your
> > description of the patch:
> >
> > > Declarations with internal linkage at file scope in the first input
> > > file get DECL_ASSEMBLER_NAME set equal to DECL_NAME.
> >
> > So, in a.c, 'x' will get DECL_ASSEMBLER_NAME of 'x', since a.c is the
> > first input file.
> >
> > Now, I presume that declarations with external linkage will continue
> > to get their own name, too, so in b.c and c.c, 'x' will also get
> > DECL_ASSEMBLER_NAME of 'x'.  (They'd better have the same name, since
> > they are the same object.)
> >
> > This is a problem, since the 'x' in a.c and the one in c.c are
> > different objects.  I expect that this code won't compile under IMA;
> > the assembler will choke.
>
> Thanks for the explanation.  I figured out what you meant about 15
> minutes after sending the previous message, but at that point I had
> already left the house and it was too late to say so.  (Had to spend
> all day helping friends move.)
>
> You're right, this won't work.  I've thought of three possible fixes.
> All entail reverting most of the behavior change in
> lhd_set_decl_assembler_name (but I'd prefer to keep the bit where we
> use the DECL_UID for the disambiguating suffix.)
>
> 1) Put back c_static_assembler_name.  Hope that all the places where
>    DECL_CONTEXT wasn't getting set right have been found.
>
> 2) Don't put back c_static_assembler_name, instead have pop_scope give
>    !TREE_PUBLIC file-scope decls a NULL DECL_CONTEXT when there's only
>    one translation unit in play, thus getting backward-compatible
>    behavior from the default hook.  Again we have to hope that all the
>    places where DECL_CONTEXT wasn't getting set right have been found.
>
> 3) Again don't put back c_static_assembler_name.  Have
>    c_write_global_declarations scan all declarations in all file
>    scopes and the externals scope, and clear DECL_CONTEXT for all
>    !TREE_PUBLIC file-scope decls except those which actually need
>    disambiguation (i.e. either two !TREE_PUBLIC decls with the same
>    name appear in different translation units, or a !TREE_PUBLIC decl
>    appears in one TU and a TREE_PUBLIC decl with the same name appears
>    in the external scope).  This guarantees that DECL_CONTEXT is
>    correct for everything, but does involve adding a big chunk of
>    code, two more iterations over all file-scope declarations, and
>    probably using tree_common flags on IDENTIFIER_NODE, which we're
>    trying to get away from.
>
> I think that all the places where DECL_CONTEXT wasn't getting set
> right have in fact been found, so I currently prefer option 2.  What
> do you think?

I considered pretty much those exact options when I was writing the 
patch that created c_static_assembler_name.  I rejected option (3) as 
an unacceptable performance penalty in the single-file case.  I 
rejected option (2) because it adds extra paths in the C frontend which 
would be very lightly tested---to fully test the compiler, you would 
have to create an extra dejagnu C torture option that adds a trivial .c 
file to the compile command.

So I chose option (1), since it had no visible disadvantages.  I still 
think it doesn't, see below.


So, there's one thing I'm trying to understand.  You write:

> The problem with all this is that the C front end's override hook only
> works properly if every declaration with internal linkage has its
> DECL_CONTEXT set at the point when the hook is called.

What do you mean by 'properly'?  In particular, what

> synthetic declarations created by language-independent code

do you have that are bound to a particular translation unit?  
Language-independent code, by definition, does not know about 
translation units; not all languages have them like C, not even all 
languages that GCC compiles, consider Fortran.  So far as I know, all 
language-independent code:

1. Creates exactly one of whatever it creates, and therefore can safely 
use NULL DECL_CONTEXT; or
2. Creates many instances and has some way of naming them uniquely 
(like a counter) and therefore can also safely use NULL DECL_CONTEXT.

So I think that your statement about the C front end's override hook is 
not correct.

Related to this, your option (3) assumes that all declarations are 
available to c_write_global_declarations.  I do not know that is a safe 
assumption; the code that adds DECL_CONTEXT is the same code that adds 
declarations into the lists that c_static_assembler_name traverses, and 
I don't think it gets called for 'synthetic declarations'.  It was not 
a design requirement for c_write_global_declarations as I originally 
wrote it; indeed, I think I assumed the reverse, but I don't think it 
made any difference.


As a final question, is there something that you're not telling us?  
Based on the changes you've been making to the C frontend recently, it 
looks like you're working on some significant enhancement or new 
feature, because otherwise it makes no sense to make them; you seem to 
be spending a lot of time breaking things and fixing them again with 
little visible benefit.  If you don't want to discuss it on the list, 
it might help if you sent me private e-mail or even telephoned me.