On Tue, Sep 16, 2025 at 10:30 AM Eric Botcazou <[email protected]> wrote: > > > I mean TREE_READONLY on ..._REF nodes. We can't rely on the absence of > > TREE_READONLY on ..._REF meaning the object is writable, so the flag does > > not add any information (but maybe some costing hint that the object is > > definitely _not_ writable(?)). > > OK, I agree that it may not say much for writability of references, but it is > tested by tree_invariant_p_1 and skip_simple_arithmetic, so it avoids useless > SAVE_EXPRs and temporaries at least. > > > The above snippet from tree-ssa-phiopt.cc (cselim) also hints at that seeing > > a write LHS where tree_could_trap_p () is false does not mean the object is > > writable. > > The comment about rvalues there is strange for sure...
It's meant to say it that tree_could_trap_p doesn't consider a write to a readonly object as a possible trap as it does not have the required context (whether the ref is on the LHS of an assignment). gimple_could_trap_p has (but mishandles this case), and tree_could_trap_p could, if you'd feed it a MODIFY_EXPR, but that case isn't handled at all. > > This relies on tree_could_trap_p () only ever returning false > > for DECL-based accesses or accesses with TREE_THIS_NOTRAP - but it > > fails to consider the latter for the readonly check. > > Yes, it acknowledges (with a questionable comment) that TREE_THIS_NOTRAP is > not sufficient and, therefore, tests TREE_READONLY on a DECL, which is a good > approximation. > > > I also wonder whether for TREE_THIS_NOTRAP non-DECL-based accesses a > > TREE_READONLY flag is required when the object accessed _might_ be readonly. > > Consider > > > > if (readonly) > > ptr = &readonly_decl; > > else > > ptr = &writable_decl; > > if (!readonly) > > *ptr = 1; > > > > if *ptr has TREE_THIS_NOTRAP set, what would the presence or absence > > of TREE_READONLY tell us? If it doesn't have TREE_THIS_NOTRAP set, > > same question. > > Given the aforementioned usage of TREE_READONLY, you cannot set it if the > value may change so, if we want to do something about this affair, I think > that we should work on TREE_THIS_NOTRAP and leave TREE_READONLY alone. Agreed. Richard. > -- > Eric Botcazou > >
