diff mbox

Fix 70199

Message ID 56E78500.1040309@redhat.com
State New
Headers show

Commit Message

Richard Henderson March 15, 2016, 3:44 a.m. UTC
The problem here is that

   void* labels[] = {
     &&l0, &&l1, &&l2
   };

gets gimplified to

   labels = *.LC0;

but .LC0 is not in the set of local decls, so that when copy_forbidden is 
called during sra versioning we fail to forbid the copy.  We could set a 
different flag, but I think it's easiest to just add the artificial decl to 
where it can be seen.

Ok?


r~
* gimplify.c (gimplify_init_constructor): Add the constant def decl
	to the function's local decls.

Comments

Richard Biener March 15, 2016, 2:13 p.m. UTC | #1
On Tue, Mar 15, 2016 at 4:44 AM, Richard Henderson <rth@redhat.com> wrote:
> The problem here is that
>
>   void* labels[] = {
>     &&l0, &&l1, &&l2
>   };
>
> gets gimplified to
>
>   labels = *.LC0;
>
> but .LC0 is not in the set of local decls, so that when copy_forbidden is
> called during sra versioning we fail to forbid the copy.  We could set a
> different flag, but I think it's easiest to just add the artificial decl to
> where it can be seen.
>
> Ok?

Hmm.  tree_output_constant_def uses the global constant pool (and not
function-scope statics).  So while for the above case with local labels
there can be no sharing and thus the decl is really "local" with non-local
labels or with other random initializers you'd have the ctor decl in
multiple local decl vectors.  Not sure if that's a problem, but at least
if you'd have

  void* labels[] = {
    &&l0, &&l1, &&l2
  };
  void* labels2[] = {
    &&l0, &&l1, &&l2
  };

you'll end up with the same constant pool decl in local-decls twice.  Given
cross-function sharing is generally possible doing the addition to local-decls
only if we create a new constant pool entry isn't enough either.  It's also
a bit pre-mature in the gimplifier as we only add to local-decls during
BIND expr lowering.

I also wonder if outputting the constant pool decl far away from the labels
might end up with invalid asm for some targets.

Well, I don't see any convenient way of fixing things here either but maybe
we can do

  if (walk_tree_without_duplicataes (&DECL_INITIAL (ctor),
has_label_address_in_static_1, cfun->decl))
    add_local_decl (cfun, ctor);

to avoid adding the decl when it is not necessary.  Having another
struct function
flag would be possible as well, or re-use has_nonlocal_label as clearly a global
static is now refering to a local label (you'd lose optimization when
'labels' becomes
unused of course).

Thanks,
Richard.

>
> r~
diff mbox

Patch

diff --git a/gcc/gimplify.c b/gcc/gimplify.c
index b331e41..884482e 100644
--- a/gcc/gimplify.c
+++ b/gcc/gimplify.c
@@ -4016,6 +4016,7 @@  gimplify_init_constructor (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p,
 
 		walk_tree (&ctor, force_labels_r, NULL, NULL);
 		ctor = tree_output_constant_def (ctor);
+		add_local_decl (cfun, ctor);
 		if (!useless_type_conversion_p (type, TREE_TYPE (ctor)))
 		  ctor = build1 (VIEW_CONVERT_EXPR, type, ctor);
 		TREE_OPERAND (*expr_p, 1) = ctor;