> 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... > 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. -- Eric Botcazou