* Re: [PATCH] Fix type field walking in gimplifier unsharing
[not found] ` <alpine.LSU.2.11.1604281249090.13384@t29.fhfr.qr>
@ 2016-04-28 12:10 ` Richard Biener
2016-04-29 8:15 ` Eric Botcazou
0 siblings, 1 reply; 3+ messages in thread
From: Richard Biener @ 2016-04-28 12:10 UTC (permalink / raw)
To: Eric Botcazou; +Cc: gcc-patches, jason, fortran
On Thu, 28 Apr 2016, Richard Biener wrote:
> On Thu, 28 Apr 2016, Eric Botcazou wrote:
>
> > > Aww, I was hoping for sth that would not require me to fix all
> > > frontends ...
> >
> > I don't really see how this can work without DECL_EXPR though. You need to
> > define when the variable-sized expressions are evaluated to lay out the type,
> > otherwise it will be laid out on the first use, which may see a different
> > value of the expressions than the definition point. The only way to do that
> > for a locally-defined type is to add a DECL_EXPR in GENERIC, so that the
> > gimplifier evaluates the expressions at the right spot.
>
> Ah, so the C++ FE does this correctly but in addition to that it has
>
> /* When the pointed-to type involves components of variable
> size,
> care must be taken to ensure that the size evaluation code is
> emitted early enough to dominate all the possible later uses
> and late enough for the variables on which it depends to have
> been assigned.
>
> This is expected to happen automatically when the pointed-to
> type has a name/declaration of it's own, but special
> attention
> is required if the type is anonymous.
> ...
> if (!TYPE_NAME (type)
> && (decl_context == NORMAL || decl_context == FIELD)
> && at_function_scope_p ()
> && variably_modified_type_p (type, NULL_TREE))
> /* Force evaluation of the SAVE_EXPR. */
> finish_expr_stmt (TYPE_SIZE (type));
>
> so in this case the type doesn't have an associated TYPE_DECL and thus
> we can't build a DECL_EXPR. To me the correct fix is then to
> always force a TYPE_DECL for variable-modified types.
>
> Jason?
The following works (for the testcase):
Index: gcc/cp/decl.c
===================================================================
--- gcc/cp/decl.c (revision 235547)
+++ gcc/cp/decl.c (working copy)
@@ -10393,8 +10393,11 @@ grokdeclarator (const cp_declarator *dec
&& (decl_context == NORMAL || decl_context == FIELD)
&& at_function_scope_p ()
&& variably_modified_type_p (type, NULL_TREE))
- /* Force evaluation of the SAVE_EXPR. */
- finish_expr_stmt (TYPE_SIZE (type));
+ {
+ TYPE_NAME (type) = build_decl (UNKNOWN_LOCATION, TYPE_DECL,
+ NULL_TREE, type);
+ add_decl_expr (TYPE_NAME (type));
+ }
if (declarator->kind == cdk_reference)
{
and I have a similar fix for the Fortran FE for one testcase I
reduced to
character(10), dimension (2) :: implicit_result
character(10), dimension (2) :: source
implicit_result = reallocate_hnv (LEN (source))
contains
FUNCTION reallocate_hnv(LEN)
CHARACTER(LEN=LEN), DIMENSION(:), POINTER :: reallocate_hnv
END FUNCTION reallocate_hnv
end
Index: fortran/trans-array.c
===================================================================
--- fortran/trans-array.c (revision 235547)
+++ fortran/trans-array.c (working copy)
@@ -1094,6 +1094,16 @@ gfc_trans_create_temp_array (stmtblock_t
info->descriptor = desc;
size = gfc_index_one_node;
+ /* Emit a DECL_EXPR for the variable sized array type in
+ GFC_TYPE_ARRAY_DATAPTR_TYPE so the gimplification of its type
+ sizes works correctly. */
+ tree arraytype = TREE_TYPE (GFC_TYPE_ARRAY_DATAPTR_TYPE (type));
+ if (! TYPE_NAME (arraytype))
+ TYPE_NAME (arraytype) = build_decl (UNKNOWN_LOCATION, TYPE_DECL,
+ NULL_TREE, arraytype);
+ gfc_add_expr_to_block (pre, build1 (DECL_EXPR,
+ arraytype, TYPE_NAME (arraytype)));
+
/* Fill in the array dtype. */
tmp = gfc_conv_descriptor_dtype (desc);
gfc_add_modify (pre, tmp, gfc_get_dtype (TREE_TYPE (desc)));
I wonder if we can avoid allocating the TYPE_DECL by simply also
allowing TREE_TYPE as operand of a DECL_EXPR (to avoid adding
a 'TYPE_EXPR').
Richard.
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] Fix type field walking in gimplifier unsharing
2016-04-28 12:10 ` [PATCH] Fix type field walking in gimplifier unsharing Richard Biener
@ 2016-04-29 8:15 ` Eric Botcazou
2016-04-29 8:18 ` Richard Biener
0 siblings, 1 reply; 3+ messages in thread
From: Eric Botcazou @ 2016-04-29 8:15 UTC (permalink / raw)
To: Richard Biener; +Cc: gcc-patches, jason, fortran
> The following works (for the testcase):
>
> Index: gcc/cp/decl.c
> ===================================================================
> --- gcc/cp/decl.c (revision 235547)
> +++ gcc/cp/decl.c (working copy)
> @@ -10393,8 +10393,11 @@ grokdeclarator (const cp_declarator *dec
> && (decl_context == NORMAL || decl_context == FIELD)
> && at_function_scope_p ()
> && variably_modified_type_p (type, NULL_TREE))
> - /* Force evaluation of the SAVE_EXPR. */
> - finish_expr_stmt (TYPE_SIZE (type));
> + {
> + TYPE_NAME (type) = build_decl (UNKNOWN_LOCATION, TYPE_DECL,
> + NULL_TREE, type);
> + add_decl_expr (TYPE_NAME (type));
> + }
>
> if (declarator->kind == cdk_reference)
> {
>
> and I have a similar fix for the Fortran FE for one testcase I
> reduced to
>
> character(10), dimension (2) :: implicit_result
> character(10), dimension (2) :: source
> implicit_result = reallocate_hnv (LEN (source))
> contains
> FUNCTION reallocate_hnv(LEN)
> CHARACTER(LEN=LEN), DIMENSION(:), POINTER :: reallocate_hnv
> END FUNCTION reallocate_hnv
> end
>
> Index: fortran/trans-array.c
> ===================================================================
> --- fortran/trans-array.c (revision 235547)
> +++ fortran/trans-array.c (working copy)
> @@ -1094,6 +1094,16 @@ gfc_trans_create_temp_array (stmtblock_t
> info->descriptor = desc;
> size = gfc_index_one_node;
>
> + /* Emit a DECL_EXPR for the variable sized array type in
> + GFC_TYPE_ARRAY_DATAPTR_TYPE so the gimplification of its type
> + sizes works correctly. */
> + tree arraytype = TREE_TYPE (GFC_TYPE_ARRAY_DATAPTR_TYPE (type));
> + if (! TYPE_NAME (arraytype))
> + TYPE_NAME (arraytype) = build_decl (UNKNOWN_LOCATION, TYPE_DECL,
> + NULL_TREE, arraytype);
> + gfc_add_expr_to_block (pre, build1 (DECL_EXPR,
> + arraytype, TYPE_NAME (arraytype)));
> +
> /* Fill in the array dtype. */
> tmp = gfc_conv_descriptor_dtype (desc);
> gfc_add_modify (pre, tmp, gfc_get_dtype (TREE_TYPE (desc)));
Great. We do exactly that in the Ada compiler (but of course the number of
places where we need to do it is an order of magnitude larger).
> I wonder if we can avoid allocating the TYPE_DECL by simply also
> allowing TREE_TYPE as operand of a DECL_EXPR (to avoid adding
> a 'TYPE_EXPR').
I agree that DECL_EXPR + TYPE_DECL is a bit heavy, but I'm not sure that the
benefit would be worth introducing the irregularity in the IL.
--
Eric Botcazou
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] Fix type field walking in gimplifier unsharing
2016-04-29 8:15 ` Eric Botcazou
@ 2016-04-29 8:18 ` Richard Biener
0 siblings, 0 replies; 3+ messages in thread
From: Richard Biener @ 2016-04-29 8:18 UTC (permalink / raw)
To: Eric Botcazou; +Cc: gcc-patches, jason, fortran
On Fri, 29 Apr 2016, Eric Botcazou wrote:
> > The following works (for the testcase):
> >
> > Index: gcc/cp/decl.c
> > ===================================================================
> > --- gcc/cp/decl.c (revision 235547)
> > +++ gcc/cp/decl.c (working copy)
> > @@ -10393,8 +10393,11 @@ grokdeclarator (const cp_declarator *dec
> > && (decl_context == NORMAL || decl_context == FIELD)
> > && at_function_scope_p ()
> > && variably_modified_type_p (type, NULL_TREE))
> > - /* Force evaluation of the SAVE_EXPR. */
> > - finish_expr_stmt (TYPE_SIZE (type));
> > + {
> > + TYPE_NAME (type) = build_decl (UNKNOWN_LOCATION, TYPE_DECL,
> > + NULL_TREE, type);
> > + add_decl_expr (TYPE_NAME (type));
> > + }
> >
> > if (declarator->kind == cdk_reference)
> > {
> >
> > and I have a similar fix for the Fortran FE for one testcase I
> > reduced to
> >
> > character(10), dimension (2) :: implicit_result
> > character(10), dimension (2) :: source
> > implicit_result = reallocate_hnv (LEN (source))
> > contains
> > FUNCTION reallocate_hnv(LEN)
> > CHARACTER(LEN=LEN), DIMENSION(:), POINTER :: reallocate_hnv
> > END FUNCTION reallocate_hnv
> > end
> >
> > Index: fortran/trans-array.c
> > ===================================================================
> > --- fortran/trans-array.c (revision 235547)
> > +++ fortran/trans-array.c (working copy)
> > @@ -1094,6 +1094,16 @@ gfc_trans_create_temp_array (stmtblock_t
> > info->descriptor = desc;
> > size = gfc_index_one_node;
> >
> > + /* Emit a DECL_EXPR for the variable sized array type in
> > + GFC_TYPE_ARRAY_DATAPTR_TYPE so the gimplification of its type
> > + sizes works correctly. */
> > + tree arraytype = TREE_TYPE (GFC_TYPE_ARRAY_DATAPTR_TYPE (type));
> > + if (! TYPE_NAME (arraytype))
> > + TYPE_NAME (arraytype) = build_decl (UNKNOWN_LOCATION, TYPE_DECL,
> > + NULL_TREE, arraytype);
> > + gfc_add_expr_to_block (pre, build1 (DECL_EXPR,
> > + arraytype, TYPE_NAME (arraytype)));
> > +
> > /* Fill in the array dtype. */
> > tmp = gfc_conv_descriptor_dtype (desc);
> > gfc_add_modify (pre, tmp, gfc_get_dtype (TREE_TYPE (desc)));
>
> Great. We do exactly that in the Ada compiler (but of course the number of
> places where we need to do it is an order of magnitude larger).
>
> > I wonder if we can avoid allocating the TYPE_DECL by simply also
> > allowing TREE_TYPE as operand of a DECL_EXPR (to avoid adding
> > a 'TYPE_EXPR').
>
> I agree that DECL_EXPR + TYPE_DECL is a bit heavy, but I'm not sure that the
> benefit would be worth introducing the irregularity in the IL.
Not sure either. I'll add a helper like build_decl_expr_for_type
that does the magic (if TYPE_NAME is NULL). That at least reduces
the amount of code duplication.
Richard.
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2016-04-29 8:18 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <alpine.LSU.2.11.1604271030510.13384@t29.fhfr.qr>
[not found] ` <1563866.3PAXIoXYk7@polaris>
[not found] ` <alpine.LSU.2.11.1604271221530.13384@t29.fhfr.qr>
[not found] ` <3302327.fD05QFZoa0@polaris>
[not found] ` <alpine.LSU.2.11.1604281249090.13384@t29.fhfr.qr>
2016-04-28 12:10 ` [PATCH] Fix type field walking in gimplifier unsharing Richard Biener
2016-04-29 8:15 ` Eric Botcazou
2016-04-29 8:18 ` 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).