* 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).