This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: IMA: backward-compatible behavior fromlhd_set_decl_assembler_name
Geoff Keating <geoffk@geoffk.org> writes:
> 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 agree there.
> 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.
I don't see how (1) and (2) are different here -- it's the same
lightly-tested extra path, but in a different place. To verify this I
implemented (2) -- patch is attached; most of the bulk is reverting
the ill-advised parts of the previous patch -- and indeed it was a
matter of adding one clause to the controlling expression of an
if-statement that already existed.
I do agree that we've got a lightly-tested code path here, but I mean
to fix that by adding an IMA test suite at the same time that I
actually turn IMA back on (hopefully that will be tomorrow).
> 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.
Ah, but no. Mudflap, for instance, *thinks* it is creating one of its
things per object file, but because mudflap_finish_file is being
called from c_objc_common_finish_file, it is actually creating one of
its things per input translation unit, and since they all get the same
symbol name, the assembler barfs.
The cure for this, however, is to move the call to mudflap_finish_file
to a point where it will only be called once per object file - that's
the next patch in my queue, once we get this current issue resolved.
In principle you are correct that language-independent code can safely
use NULL DECL_CONTEXT - which is why I said in my original message
that I thought (2) was now safe.
The bulk of the work I have been doing to date is to stop the C front
end - language *dependent* code - from instantiating
DECL_ASSEMBLER_NAME too early, when DECL_CONTEXT for file-scope
statics is still null merely because the TRANSLATION_UNIT_DECL hasn't
been created yet.
> 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.
Nope. I am doing no more and no less than is necessary to fix IMA,
and you yourself warned me about most of the problems I have been
fixing, the ones where DECL_ASSEMBLER_NAME would be set too early.
Some of the patches should have compile-time performance benefits
independent of IMA, but that's not a current goal of mine (my
management has instructed me to finish fixing IMA first).
Here's my current candidate patch. It gets no regressions on
i686-linux and I've checked a few object files to make sure the symbol
names are correct.
zw
* c-decl.c (pop_scope): Do not set DECL_CONTEXT on file-scope
decls when there is only one input translation unit.
* langhooks.c (lhd_set_decl_assembler_name): Partially revert
change of 2004-07-05; do not treat declarations with
DECL_CONTEXT a TRANSLATION_UNIT_DECL specially.
* opts.c (cur_in_fname): Delete.
* opts.h: Likewise.
* tree.c: Revert changes of 2004-07-05; no special treatment
for TRANSLATION_UNIT_DECLs.
* Makefile.in (tree.o): Update dependencies.
===================================================================
Index: Makefile.in
--- Makefile.in 6 Jul 2004 16:28:29 -0000 1.1320
+++ Makefile.in 7 Jul 2004 19:39:38 -0000
@@ -1569,7 +1569,7 @@ langhooks.o : langhooks.c $(CONFIG_H) $(
$(LANGHOOKS_DEF_H) $(FLAGS_H) $(GGC_H) diagnostic.h
tree.o : tree.c $(CONFIG_H) $(SYSTEM_H) coretypes.h $(TM_H) $(TREE_H) $(FLAGS_H) function.h \
toplev.h $(GGC_H) $(HASHTAB_H) $(TARGET_H) output.h $(TM_P_H) langhooks.h \
- real.h gt-tree.h tree-iterator.h $(BASIC_BLOCK_H) $(TREE_FLOW_H) opts.h
+ real.h gt-tree.h tree-iterator.h $(BASIC_BLOCK_H) $(TREE_FLOW_H)
tree-dump.o: tree-dump.c $(CONFIG_H) $(SYSTEM_H) coretypes.h $(TM_H) $(TREE_H) \
$(C_TREE_H) $(FLAGS_H) langhooks.h toplev.h output.h c-pragma.h $(RTL_H) \
$(GGC_H) $(EXPR_H) $(SPLAY_TREE_H) $(TREE_DUMP_H) tree-iterator.h
===================================================================
Index: c-decl.c
--- c-decl.c 6 Jul 2004 02:20:05 -0000 1.528
+++ c-decl.c 7 Jul 2004 19:39:38 -0000
@@ -760,10 +760,12 @@ pop_scope (void)
TREE_CHAIN (p) = BLOCK_VARS (block);
BLOCK_VARS (block) = p;
}
- /* If this is the file scope, must set DECL_CONTEXT on
- these. Do so even for externals, so that
- same_translation_unit_p works. */
- if (scope == file_scope)
+ /* If this is the file scope, and we are processing more
+ than one translation unit in this compilation, set
+ DECL_CONTEXT of each decl to the TRANSLATION_UNIT_DECL.
+ This makes same_translation_unit_p work, and causes
+ static declarations to be given disambiguating suffixes. */
+ if (scope == file_scope && num_in_fnames > 1)
DECL_CONTEXT (p) = context;
/* Fall through. */
===================================================================
Index: langhooks.c
--- langhooks.c 5 Jul 2004 17:28:33 -0000 1.66
+++ langhooks.c 7 Jul 2004 19:39:38 -0000
@@ -179,31 +179,15 @@ lhd_set_decl_assembler_name (tree decl)
Can't use just the variable's own name for a variable whose
scope is less than the whole compilation. Concatenate a
- distinguishing number. If the decl is at block scope, the
- number assigned is the DECL_UID; if the decl is at file
- scope, the number is the DECL_UID of the surrounding
- TRANSLATION_UNIT_DECL, except for the T_U_D with UID 0.
- Those (the file-scope internal-linkage declarations from the
- first input file) get no suffix, which is consistent with
- what has historically been done for file-scope declarations
- with internal linkage. */
- if (TREE_PUBLIC (decl)
- || DECL_CONTEXT (decl) == NULL_TREE
- || (TREE_CODE (DECL_CONTEXT (decl)) == TRANSLATION_UNIT_DECL
- && DECL_UID (DECL_CONTEXT (decl)) == 0))
+ distinguishing number - we use the DECL_UID. */
+ if (TREE_PUBLIC (decl) || DECL_CONTEXT (decl) == NULL_TREE)
SET_DECL_ASSEMBLER_NAME (decl, DECL_NAME (decl));
else
{
const char *name = IDENTIFIER_POINTER (DECL_NAME (decl));
char *label;
- unsigned int uid;
- if (TREE_CODE (DECL_CONTEXT (decl)) == TRANSLATION_UNIT_DECL)
- uid = DECL_UID (DECL_CONTEXT (decl));
- else
- uid = DECL_UID (decl);
-
- ASM_FORMAT_PRIVATE_NAME (label, name, uid);
+ ASM_FORMAT_PRIVATE_NAME (label, name, DECL_UID (decl));
SET_DECL_ASSEMBLER_NAME (decl, get_identifier (label));
}
}
===================================================================
Index: opts.c
--- opts.c 5 Jul 2004 17:28:33 -0000 1.72
+++ opts.c 7 Jul 2004 19:39:38 -0000
@@ -92,7 +92,6 @@ static bool flag_peel_loops_set, flag_br
/* Input file names. */
const char **in_fnames;
unsigned num_in_fnames;
-unsigned cur_in_fname;
static size_t find_opt (const char *, int);
static int common_handle_option (size_t scode, const char *arg, int value);
===================================================================
Index: opts.h
--- opts.h 5 Jul 2004 17:28:33 -0000 1.15
+++ opts.h 7 Jul 2004 19:39:38 -0000
@@ -57,8 +57,4 @@ extern const char **in_fnames;
extern unsigned num_in_fnames;
-/* Current input filename index. */
-
-extern unsigned cur_in_fname;
-
#endif
===================================================================
Index: tree.c
--- tree.c 6 Jul 2004 02:20:09 -0000 1.389
+++ tree.c 7 Jul 2004 19:39:40 -0000
@@ -48,7 +48,6 @@ Software Foundation, 59 Temple Place - S
#include "tree-iterator.h"
#include "basic-block.h"
#include "tree-flow.h"
-#include "opts.h"
/* obstack.[ch] explicitly declined to prototype this. */
extern int _obstack_allocated_p (struct obstack *h, void *obj);
@@ -310,10 +309,7 @@ make_node_stat (enum tree_code code MEM_
DECL_USER_ALIGN (t) = 0;
DECL_IN_SYSTEM_HEADER (t) = in_system_header;
DECL_SOURCE_LOCATION (t) = input_location;
- if (code == TRANSLATION_UNIT_DECL)
- DECL_UID (t) = cur_in_fname;
- else
- DECL_UID (t) = next_decl_uid++;
+ DECL_UID (t) = next_decl_uid++;
/* We have not yet computed the alias set for this declaration. */
DECL_POINTER_ALIAS_SET (t) = -1;
@@ -386,7 +382,7 @@ copy_node_stat (tree node MEM_STAT_DECL)
TREE_VISITED (t) = 0;
t->common.ann = 0;
- if (TREE_CODE_CLASS (code) == 'd' && code != TRANSLATION_UNIT_DECL)
+ if (TREE_CODE_CLASS (code) == 'd')
DECL_UID (t) = next_decl_uid++;
else if (TREE_CODE_CLASS (code) == 't')
{
@@ -5341,14 +5337,6 @@ make_or_reuse_type (unsigned size, int u
void
build_common_tree_nodes (int signed_char)
{
- /* This function is called after command line parsing is complete,
- but before any DECL nodes should have been created. Therefore,
- now is the appropriate time to adjust next_decl_uid so that the
- range 0 .. num_in_fnames-1 is reserved for TRANSLATION_UNIT_DECLs. */
- if (next_decl_uid)
- abort ();
- next_decl_uid = num_in_fnames;
-
error_mark_node = make_node (ERROR_MARK);
TREE_TYPE (error_mark_node) = error_mark_node;