Re: [patch] Introduce deep unsharing

[Richard Guenther <richard dot guenther at gmail dot com>]

On Thu, May 13, 2010 at 11:15 AM, Eric Botcazou <ebotcazou@adacore.com> wrote:
> Hi,
>
> in Ada, sizes of types can be arbitrarily complex expressions. ?When a type
> declared at library (i.e. global) level with complex size expressions is
> referenced in a function, the expressions can become part of the GENERIC
> representation of the function. ?Without specific counter-measures, and since
> gimplification is done in-place, temporaries created by the gimplifier in the
> middle of these expressions would end up attached to these expressions, which
> means that if a second function references the same type, it can inherit a
> temporary created in the first function. ?That's of course invalid.
>
> The counter-measure is tree unsharing: prior to gimplification, the GENERIC
> representation of the functions is "unshared", which means that tree nodes
> referenced more than once in the function are duplicated. ?For the global
> case, the trick is to "mark" the tree nodes coming from the size expressions
> when they are created, so that they are duplicated if they are referenced
> just once in the function.

Hum.  But If you have a global type that has a complex size expression
you will have its TYPE_SIZES_GIMPLIFIED flag set after the first
function using it has been gimplified.  Won't this cause problems for any
further functions?  Or am I missing something?

> This works fine, except when there are SAVE_EXPRs or TARGET_EXPRs in the trees
> because these nodes are special: they cannot be duplicated. ?So the recursive
> duplication mechanism stops at them. ?Problems happen when size expressions
> for global types are referenced behind these SAVE_EXPRs or TARGET_EXPRs, as
> nothing will duplicate the nodes there.

So I can't see how SAVE_EXPRs or TARGET_EXPRs are any special to the problem?

Richard.

> 99% of the problematic cases occur in "elaboration procedures", the equivalent
> of global constructors in C++, so we have the following kluge in gigi:
>
> ?/* Finally see if we have any elaboration procedures to deal with. ?*/
> ?for (info = elab_info_list; info; info = info->next)
> ? ?{
> ? ? ?tree gnu_body = DECL_SAVED_TREE (info->elab_proc), gnu_stmts;
>
> ? ? ?/* Unshare SAVE_EXPRs between subprograms. ?These are not unshared by
> ? ? ? ? the gimplifier for obvious reasons, but it turns out that we need to
> ? ? ? ? unshare them for the global level because of SAVE_EXPRs made around
> ? ? ? ? checks for global objects and around allocators for global objects
> ? ? ? ? of variable size, in order to prevent node sharing in the underlying
> ? ? ? ? expression. ?Note that this implicitly assumes that the SAVE_EXPR
> ? ? ? ? nodes themselves are not shared between subprograms, which would be
> ? ? ? ? an upstream bug for which we would not change the outcome. ?*/
> ? ? ?walk_tree_without_duplicates (&gnu_body, unshare_save_expr, NULL);
>
> The attached testcase discr23.adb is one of those.
>
> But nothing prevents the problem from arising in regular procedures as well,
> as shown by the second attached testcase controlled1.ads which crashes the
> unmodified compiler. ?So, in Ada, we need to unshare the tree nodes behind
> SAVE_EXPRs (and TARGET_EXPRs but Ada doesn't generate TARGET_EXPRs) in all
> functions.
>
> Hence the attached patch, which introduces "deep unsharing", controlled by a
> language hook since I don't think other languages necessarily want it too.
>
> Tested on i586-suse-linux and x86_64-suse-linux, OK for mainline?
>
>
> 2010-05-13 ?Eric Botcazou ?<ebotcazou@adacore.com>
>
> ? ? ? ?* langhooks.h (struct lang_hooks): Add new field deep_unsharing.
> ? ? ? ?* langhooks-def.h (LANG_HOOKS_DEEP_UNSHARING): New macro.
> ? ? ? ?(LANG_HOOKS_INITIALIZER): Add LANG_HOOKS_DEEP_UNSHARING.
> ? ? ? ?* gimplify.c: (mostly_copy_tree_r): Copy trees under SAVE_EXPR and
> ? ? ? ?TARGET_EXPR nodes, but only once, if instructed to do so. ?Do not
> ? ? ? ?propagate the 'data' argument to copy_tree_r.
> ? ? ? ?(copy_if_shared_r): Remove bogus ATTRIBUTE_UNUSED marker.
> ? ? ? ?Propagate 'data' argument to walk_tree.
> ? ? ? ?(copy_if_shared): New function.
> ? ? ? ?(unmark_visited_r): Remove bogus ATTRIBUTE_UNUSED marker.
> ? ? ? ?(unmark_visited): New function.
> ? ? ? ?(unshare_body): Call copy_if_shared instead of doing it manually.
> ? ? ? ?(unvisit_body): Call unmark_visited instead of doing it manually.
> ada/
> ? ? ? ?* gcc-interface/misc.c (LANG_HOOKS_DEEP_UNSHARING): Redefine.
> ? ? ? ?* gcc-interface/trans.c (unshare_save_expr): Delete.
> ? ? ? ?(gigi): Do not unshare trees under SAVE_EXPRs here.
>
>
> 2010-05-13 ?Eric Botcazou ?<ebotcazou@adacore.com>
>
> ? ? ? ?* gnat.dg/discr23.ad[sb]: New test.
> ? ? ? ?* gnat.dg/discr23_pkg.ads: New helper.
>
> ? ? ? ?* gnat.dg/specs/controlled1.ads: New test.
> ? ? ? ?* gnat.dg/specs/controlled1_pkg.ads: New helper.
>
>
> --
> Eric Botcazou
>