On Wed, 23 Jun 2021, Martin Jambor wrote:
> Hi,
>
> tree-inline leaves behind VAR_DECLs which are TREE_READONLY (because
> they are copies of const parameters) but are written to because they
> need to be initialized. This patch resets the flag if any
> initialization is performed, regardless of if the type needs
> construction or not.
I wonder if even that is premature optimization - it also removes
a still useful comment. So why not at the same place simply do
/* Even if P was TREE_READONLY, the new VAR should not be.
In the original code, we would have constructed a
temporary, and then the function body would have never
changed the value of P. However, now, we will be
constructing VAR directly. Therefore, it must not
be TREE_READONLY. */
TREE_READONLY (var) = 0;
? Since technically when in SSA form you wouldn't need to bother
either, so your placement looks both premature and not good enough
optimization.
Did you check the above suggestion already and it failed for some
reason?
Thanks,
Richard
> There are other sources of variables which are incorrectly marked as
> TREE_READOLY, but with this patch and a verifier catching them I can
> at least compile the Ada run-time library.
>
> Bootstrapped and tested on x86_64-linux and aarch64-linux, LTO bootstrap
> on x86_64-linux is ongoing. OK for trunk if it passes?
>
> Thanks,
>
> Martin
>
>
> gcc/ChangeLog:
>
> 2021-06-22 Martin Jambor <mjam...@suse.cz>
>
> * tree-inline.c (setup_one_parameter): Set TREE_READONLY of the
> param replacement to zero if it is initialized, regardless if it
> needs constructing.
> ---
> gcc/tree-inline.c | 13 +------------
> 1 file changed, 1 insertion(+), 12 deletions(-)
>
> diff --git a/gcc/tree-inline.c b/gcc/tree-inline.c
> index 4a0dc3b6b60..ac8abcd400b 100644
> --- a/gcc/tree-inline.c
> +++ b/gcc/tree-inline.c
> @@ -3491,18 +3491,6 @@ setup_one_parameter (copy_body_data *id, tree p, tree
> value, tree fn,
> automatically replaced by the VAR_DECL. */
> insert_decl_map (id, p, var);
>
> - /* Even if P was TREE_READONLY, the new VAR should not be.
> - In the original code, we would have constructed a
> - temporary, and then the function body would have never
> - changed the value of P. However, now, we will be
> - constructing VAR directly. The constructor body may
> - change its value multiple times as it is being
> - constructed. Therefore, it must not be TREE_READONLY;
> - the back-end assumes that TREE_READONLY variable is
> - assigned to only once. */
> - if (TYPE_NEEDS_CONSTRUCTING (TREE_TYPE (p)))
> - TREE_READONLY (var) = 0;
> -
> tree rhs = value;
> if (value
> && value != error_mark_node
> @@ -3549,6 +3537,7 @@ setup_one_parameter (copy_body_data *id, tree p, tree
> value, tree fn,
> return insert_init_debug_bind (id, bb, var, rhs, NULL);
> }
>
> + TREE_READONLY (var) = 0;
> STRIP_USELESS_TYPE_CONVERSION (rhs);
>
> /* If we are in SSA form properly remap the default definition
>
--
Richard Biener <rguent...@suse.de>
SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
Germany; GF: Felix Imendörffer; HRB 36809 (AG Nuernberg)
