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. 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. Martin
