[PATCH] tree-inline: Fix TREE_READONLY of parameter replacements

[Richard Biener]

On Wed, 23 Jun 2021, Martin Jambor wrote:

> Hi,
> 
> On Wed, Jun 23 2021, Richard Biener wrote:
> > 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?
> >
> 
> No, I believe the above would work fine.  I just looked at the code and
> the TREE_READONLY resetting is now just above two early exits which only
> do an insert_init_debug_bind, so I thought it made sense to move the
> reset of TREE_READONLY only to the same branch which actually generates
> a write to var.

Yeah, but we _do_ create the var and register it as copy of p earlier

  insert_decl_map (id, p, var);

so it seems consistent to adjust things whether or not there are
actual writes.

> But only because I thought it would be more consistent, and "feel" more
> logical, not as an optimization.  I do not think it makes any difference
> in practice.  So if your feelings are different, I can certainly leave
> it where it is.
> 
> As far as the comment is concerned, I like having comments about almost
> everything, but this one seemed to me a bit too obvious, especially with
> the TYPE_NEEDS_CONSTRUCTING condition removed.  But I can certainly keep
> it too.

Yes to both please.  OK with those changes.

Thanks,
Richard.