* [PATCH] Avoid invalid sharing of ADDR_EXPRs (PR fortran/55935)
@ 2013-01-11 19:47 Jakub Jelinek
2013-01-12 8:08 ` Richard Biener
0 siblings, 1 reply; 2+ messages in thread
From: Jakub Jelinek @ 2013-01-11 19:47 UTC (permalink / raw)
To: Richard Biener, fortran; +Cc: gcc-patches
Hi!
As discussed in the PR, the extra verification of location blocks Richard
posted recently fails on some Fortran testcases. The problem is that
ADDR_EXPRs in static const var initializers contain locations (fixed by the
trans-expr.c hunks), and that gimple-fold makes the ADDR_EXPRs shared, so
even with the fortran FE hunks alone when the gimplifier sets location
we get invalid blocks anyway. In the PR we were discussing putting the
unshare_expr at the beginning of canonicalize_constructor_val, unfortunately
that breaks Ada bootstrap, because it is undesirable when record_reference
calls that function. So this alternative patch moves the unshare_expr calls
to gimple-fold.c canonicalize_constructor_val callers.
Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
Alternatively the get_symbol_constant_value unshare_expr call could move
from canonicalize_constructor_val argument to return val; line, both
locations have some advantages and disadvantages (the one in patch might
create unnecessary garbage if canonicalize_constructor_val returns NULL
or non-min_invariant, the other way would create garbage with the trees
created by canonicalize_constructor_val itself (they would be created
and immediately unshared).
2013-01-11 Jakub Jelinek <jakub@redhat.com>
PR fortran/55935
* gimple-fold.c (get_symbol_constant_value): Call
unshare_expr.
(fold_gimple_assign): Don't call unshare_expr here.
(fold_ctor_reference): Call unshare_expr.
* trans-expr.c (gfc_conv_structure): Call
unshare_expr_without_location on the ctor elements.
--- gcc/gimple-fold.c.jj 2013-01-11 09:02:55.000000000 +0100
+++ gcc/gimple-fold.c 2013-01-11 15:42:52.485630537 +0100
@@ -202,7 +202,7 @@ get_symbol_constant_value (tree sym)
tree val = DECL_INITIAL (sym);
if (val)
{
- val = canonicalize_constructor_val (val, sym);
+ val = canonicalize_constructor_val (unshare_expr (val), sym);
if (val && is_gimple_min_invariant (val))
return val;
else
@@ -378,7 +378,7 @@ fold_gimple_assign (gimple_stmt_iterator
}
else if (DECL_P (rhs))
- return unshare_expr (get_symbol_constant_value (rhs));
+ return get_symbol_constant_value (rhs);
/* If we couldn't fold the RHS, hand over to the generic
fold routines. */
@@ -2941,7 +2941,7 @@ fold_ctor_reference (tree type, tree cto
/* We found the field with exact match. */
if (useless_type_conversion_p (type, TREE_TYPE (ctor))
&& !offset)
- return canonicalize_constructor_val (ctor, from_decl);
+ return canonicalize_constructor_val (unshare_expr (ctor), from_decl);
/* We are at the end of walk, see if we can view convert the
result. */
@@ -2950,7 +2950,7 @@ fold_ctor_reference (tree type, tree cto
&& operand_equal_p (TYPE_SIZE (type),
TYPE_SIZE (TREE_TYPE (ctor)), 0))
{
- ret = canonicalize_constructor_val (ctor, from_decl);
+ ret = canonicalize_constructor_val (unshare_expr (ctor), from_decl);
ret = fold_unary (VIEW_CONVERT_EXPR, type, ret);
if (ret)
STRIP_NOPS (ret);
--- gcc/fortran/trans-expr.c.jj 2013-01-11 09:02:50.000000000 +0100
+++ gcc/fortran/trans-expr.c 2013-01-11 10:43:54.071921147 +0100
@@ -6137,6 +6137,7 @@ gfc_conv_structure (gfc_se * se, gfc_exp
gfc_symbol *vtabs;
vtabs = cm->initializer->symtree->n.sym;
vtab = gfc_build_addr_expr (NULL_TREE, gfc_get_symbol_decl (vtabs));
+ vtab = unshare_expr_without_location (vtab);
CONSTRUCTOR_APPEND_ELT (v, cm->backend_decl, vtab);
}
else if (cm->ts.u.derived && strcmp (cm->name, "_size") == 0)
@@ -6150,6 +6151,7 @@ gfc_conv_structure (gfc_se * se, gfc_exp
TREE_TYPE (cm->backend_decl),
cm->attr.dimension, cm->attr.pointer,
cm->attr.proc_pointer);
+ val = unshare_expr_without_location (val);
/* Append it to the constructor list. */
CONSTRUCTOR_APPEND_ELT (v, cm->backend_decl, val);
Jakub
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [PATCH] Avoid invalid sharing of ADDR_EXPRs (PR fortran/55935)
2013-01-11 19:47 [PATCH] Avoid invalid sharing of ADDR_EXPRs (PR fortran/55935) Jakub Jelinek
@ 2013-01-12 8:08 ` Richard Biener
0 siblings, 0 replies; 2+ messages in thread
From: Richard Biener @ 2013-01-12 8:08 UTC (permalink / raw)
To: Jakub Jelinek, Jakub Jelinek, Richard Biener, fortran; +Cc: gcc-patches
Jakub Jelinek <jakub@redhat.com> wrote:
>Hi!
>
>As discussed in the PR, the extra verification of location blocks
>Richard
>posted recently fails on some Fortran testcases. The problem is that
>ADDR_EXPRs in static const var initializers contain locations (fixed by
>the
>trans-expr.c hunks), and that gimple-fold makes the ADDR_EXPRs shared,
>so
>even with the fortran FE hunks alone when the gimplifier sets location
>we get invalid blocks anyway. In the PR we were discussing putting the
>unshare_expr at the beginning of canonicalize_constructor_val,
>unfortunately
>that breaks Ada bootstrap, because it is undesirable when
>record_reference
>calls that function. So this alternative patch moves the unshare_expr
>calls
>to gimple-fold.c canonicalize_constructor_val callers.
>
>Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
Ok.
Thanks,
Richard.
>Alternatively the get_symbol_constant_value unshare_expr call could
>move
>from canonicalize_constructor_val argument to return val; line, both
>locations have some advantages and disadvantages (the one in patch
>might
>create unnecessary garbage if canonicalize_constructor_val returns NULL
>or non-min_invariant, the other way would create garbage with the trees
>created by canonicalize_constructor_val itself (they would be created
>and immediately unshared).
>
>2013-01-11 Jakub Jelinek <jakub@redhat.com>
>
> PR fortran/55935
> * gimple-fold.c (get_symbol_constant_value): Call
> unshare_expr.
> (fold_gimple_assign): Don't call unshare_expr here.
> (fold_ctor_reference): Call unshare_expr.
>
> * trans-expr.c (gfc_conv_structure): Call
> unshare_expr_without_location on the ctor elements.
>
>--- gcc/gimple-fold.c.jj 2013-01-11 09:02:55.000000000 +0100
>+++ gcc/gimple-fold.c 2013-01-11 15:42:52.485630537 +0100
>@@ -202,7 +202,7 @@ get_symbol_constant_value (tree sym)
> tree val = DECL_INITIAL (sym);
> if (val)
> {
>- val = canonicalize_constructor_val (val, sym);
>+ val = canonicalize_constructor_val (unshare_expr (val), sym);
> if (val && is_gimple_min_invariant (val))
> return val;
> else
>@@ -378,7 +378,7 @@ fold_gimple_assign (gimple_stmt_iterator
> }
>
> else if (DECL_P (rhs))
>- return unshare_expr (get_symbol_constant_value (rhs));
>+ return get_symbol_constant_value (rhs);
>
> /* If we couldn't fold the RHS, hand over to the generic
> fold routines. */
>@@ -2941,7 +2941,7 @@ fold_ctor_reference (tree type, tree cto
> /* We found the field with exact match. */
> if (useless_type_conversion_p (type, TREE_TYPE (ctor))
> && !offset)
>- return canonicalize_constructor_val (ctor, from_decl);
>+ return canonicalize_constructor_val (unshare_expr (ctor),
>from_decl);
>
> /* We are at the end of walk, see if we can view convert the
> result. */
>@@ -2950,7 +2950,7 @@ fold_ctor_reference (tree type, tree cto
> && operand_equal_p (TYPE_SIZE (type),
> TYPE_SIZE (TREE_TYPE (ctor)), 0))
> {
>- ret = canonicalize_constructor_val (ctor, from_decl);
>+ ret = canonicalize_constructor_val (unshare_expr (ctor),
>from_decl);
> ret = fold_unary (VIEW_CONVERT_EXPR, type, ret);
> if (ret)
> STRIP_NOPS (ret);
>--- gcc/fortran/trans-expr.c.jj 2013-01-11 09:02:50.000000000 +0100
>+++ gcc/fortran/trans-expr.c 2013-01-11 10:43:54.071921147 +0100
>@@ -6137,6 +6137,7 @@ gfc_conv_structure (gfc_se * se, gfc_exp
> gfc_symbol *vtabs;
> vtabs = cm->initializer->symtree->n.sym;
> vtab = gfc_build_addr_expr (NULL_TREE, gfc_get_symbol_decl (vtabs));
>+ vtab = unshare_expr_without_location (vtab);
> CONSTRUCTOR_APPEND_ELT (v, cm->backend_decl, vtab);
> }
> else if (cm->ts.u.derived && strcmp (cm->name, "_size") == 0)
>@@ -6150,6 +6151,7 @@ gfc_conv_structure (gfc_se * se, gfc_exp
> TREE_TYPE (cm->backend_decl),
> cm->attr.dimension, cm->attr.pointer,
> cm->attr.proc_pointer);
>+ val = unshare_expr_without_location (val);
>
> /* Append it to the constructor list. */
> CONSTRUCTOR_APPEND_ELT (v, cm->backend_decl, val);
>
> Jakub
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2013-01-12 8:08 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-01-11 19:47 [PATCH] Avoid invalid sharing of ADDR_EXPRs (PR fortran/55935) Jakub Jelinek
2013-01-12 8:08 ` Richard Biener
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).