This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH, PR 43257] Mangle DECL_ASSEMBLER_NAME early in IPA-SRA
On Thu, 11 Mar 2010, Martin Jambor wrote:
> Hi,
>
> On Wed, Mar 10, 2010 at 10:56:24AM +0100, Richard Guenther wrote:
> > On Tue, 9 Mar 2010, Martin Jambor wrote:
> > > the patch below makes IPA-SRA mangle the assembler name of a function
> > > decl before changing its type to avoid potential conflicts with other
> > > functions.
> > >
> > > Bootstrapped and regression tested on x86_64-linux. OK for trunk?
> >
> > Hm. Please look at tree.c:free_lang_data_in_cgraph - that seems to
> > fixup some diagnostic issues.
>
> I see.
>
> > But on the whole thing I wonder if
> > we shouldn't simply make sure that functions with a cgraph node
> > have an assembler name set in cgraph_finalize_function (or
> > cgraph_analyze_function if we want to delay it until after
> > cgraph_finalize_compilation_unit)? Thus, replicate the
> > if (need_assembler_name_p (t)) from tree.c:free_lang_data_in_cgraph
> > in cgraph_analyze_function (and maybe similar in
> > varpool_analyze_pending_decls).
> >
> > Can you try doing just the cgraph part in cgraph_analyze_function?
>
> I am not sure if I understand what you mean. Do you want just the
> declaration of the function to have its assembler name assigned at
> that time or do you think we should duplicate the whole declaration
> discovery machinery there as well? Doing the former was easy, it does
> fix the issue and the (bootstrapped and tested) patch is below. On an
> i686, the memory consumption as measured by maxmem2.sh rose only very
> slightly, from 272220 kB to 272268 kB.
Yes, that is what I had in mind (well, or doing it in
cgraph_analyze_function which would be slightly more safe I guess).
If the patch bootstrapped and tested ok with all languages enabled
(including Ada) then it is ok. If you need to re-test then
play safe and move assign_assembler_name_if_neeeded to
cgraph_analyze_function.
Thanks,
Richard.
> Thanks,
>
> Martin
>
>
> 2010-03-11 Martin Jambor <mjambor@suse.cz>
>
> PR tree-optimization/43257
> * tree.c (assign_assembler_name_if_neeeded): New function.
> (free_lang_data_in_cgraph): Assembler name assignment moved to
> the above new function.
> * tree.h (assign_assembler_name_if_neeeded): Declare.
> * cgraphunit.c (cgraph_finalize_function): Create an assembler name for
> the function if needed.
>
> * testsuite/g++.dg/torture/pr43257.C: New test.
>
> Index: mine/gcc/testsuite/g++.dg/torture/pr43257.C
> ===================================================================
> --- /dev/null
> +++ mine/gcc/testsuite/g++.dg/torture/pr43257.C
> @@ -0,0 +1,30 @@
> +/* { dg-do assemble } */
> +
> +class A {};
> +class B {};
> +
> +static void *func (int n)
> +{
> + void *p;
> + if (p == 0) throw ::A ();
> +}
> +
> +static void *func (int n, B const &)
> +{
> + try {
> + return func (n);
> + }
> + catch (::A const &) {
> + }
> + return func (n);
> +}
> +
> +void *f1 (int n)
> +{
> + return func (n, B());
> +}
> +
> +void *f2 (int n)
> +{
> + return func (n, B());
> +}
> Index: mine/gcc/cgraphunit.c
> ===================================================================
> --- mine.orig/gcc/cgraphunit.c
> +++ mine/gcc/cgraphunit.c
> @@ -523,6 +523,8 @@ cgraph_finalize_function (tree decl, boo
> if (cgraph_decide_is_function_needed (node, decl))
> cgraph_mark_needed_node (node);
>
> + assign_assembler_name_if_neeeded (node->decl);
> +
> /* Since we reclaim unreachable nodes at the end of every language
> level unit, we need to be conservative about possible entry points
> there. */
> Index: mine/gcc/tree.c
> ===================================================================
> --- mine.orig/gcc/tree.c
> +++ mine/gcc/tree.c
> @@ -4831,6 +4831,33 @@ find_decls_types_in_var (struct varpool_
> find_decls_types (v->decl, fld);
> }
>
> +/* If T needs an assembler name, have one created for it. */
> +
> +void
> +assign_assembler_name_if_neeeded (tree t)
> +{
> + if (need_assembler_name_p (t))
> + {
> + /* When setting DECL_ASSEMBLER_NAME, the C++ mangler may emit
> + diagnostics that use input_location to show locus
> + information. The problem here is that, at this point,
> + input_location is generally anchored to the end of the file
> + (since the parser is long gone), so we don't have a good
> + position to pin it to.
> +
> + To alleviate this problem, this uses the location of T's
> + declaration. Examples of this are
> + testsuite/g++.dg/template/cond2.C and
> + testsuite/g++.dg/template/pr35240.C. */
> + location_t saved_location = input_location;
> + input_location = DECL_SOURCE_LOCATION (t);
> +
> + decl_assembler_name (t);
> +
> + input_location = saved_location;
> + }
> +}
> +
>
> /* Free language specific information for every operand and expression
> in every node of the call graph. This process operates in three stages:
> @@ -4880,26 +4907,7 @@ free_lang_data_in_cgraph (void)
> now because free_lang_data_in_decl will invalidate data needed
> for mangling. This breaks mangling on interdependent decls. */
> for (i = 0; VEC_iterate (tree, fld.decls, i, t); i++)
> - if (need_assembler_name_p (t))
> - {
> - /* When setting DECL_ASSEMBLER_NAME, the C++ mangler may emit
> - diagnostics that use input_location to show locus
> - information. The problem here is that, at this point,
> - input_location is generally anchored to the end of the file
> - (since the parser is long gone), so we don't have a good
> - position to pin it to.
> -
> - To alleviate this problem, this uses the location of T's
> - declaration. Examples of this are
> - testsuite/g++.dg/template/cond2.C and
> - testsuite/g++.dg/template/pr35240.C. */
> - location_t saved_location = input_location;
> - input_location = DECL_SOURCE_LOCATION (t);
> -
> - decl_assembler_name (t);
> -
> - input_location = saved_location;
> - }
> + assign_assembler_name_if_neeeded (t);
>
> /* Traverse every decl found freeing its language data. */
> for (i = 0; VEC_iterate (tree, fld.decls, i, t); i++)
> Index: mine/gcc/tree.h
> ===================================================================
> --- mine.orig/gcc/tree.h
> +++ mine/gcc/tree.h
> @@ -4720,6 +4720,8 @@ extern tree build_low_bits_mask (tree, u
> extern tree tree_strip_nop_conversions (tree);
> extern tree tree_strip_sign_nop_conversions (tree);
> extern tree lhd_gcc_personality (void);
> +extern void assign_assembler_name_if_neeeded (tree);
> +
>
> /* In cgraph.c */
> extern void change_decl_assembler_name (tree, tree);
>
>
--
Richard Guenther <rguenther@suse.de>
Novell / SUSE Labs
SUSE LINUX Products GmbH - Nuernberg - AG Nuernberg - HRB 16746 - GF: Markus Rex