On Fri, 5 May 2023, Jakub Jelinek wrote: > On Fri, May 05, 2023 at 01:32:02PM -0400, Jason Merrill wrote: > > > --- gcc/ada/gcc-interface/utils2.cc.jj 2023-01-16 23:19:05.539727388 > > > +0100 > > > +++ gcc/ada/gcc-interface/utils2.cc 2023-05-05 15:37:30.193990948 > > > +0200 > > > @@ -3332,6 +3332,7 @@ gnat_invariant_expr (tree expr) > > > case IMAGPART_EXPR: > > > case VIEW_CONVERT_EXPR: > > > CASE_CONVERT: > > > + case SAVE_EXPR: > > > > I guess doing this would allow gnat_invariant_expr to handle > > DECL_INVARIANT_P that save_expr doesn't know about. But it seems that it > > makes the same assumption as tree_invariant_p_1 about the pointed-to object > > not changing: > > > > > case INDIRECT_REF: > > > if ((!invariant_p && !TREE_READONLY (t)) || TREE_SIDE_EFFECTS > > > (t)) > > > return NULL_TREE; > > > > I don't know if this assumption is any more valid in Ada than in C/C++. > > I think we really need Eric (as one who e.g. introduced the > DECL_INVARIANT_P apparently for this kind of stuff) to have a look at that on > the > Ada side. > > The question is if the posted tree.cc (smallest) patch + 3 new testcases > + the 7 ada testsuite workarounds are ok for trunk if it passes > bootstrap/regtest, then I'd file a PR about the Ada regression and only once > it is dealt with would consider backporting, or if we need to wait for Eric > before making progress.
I wonder if we should defer some of the choices to a langhook like make the tree_invariant_p_1 a langhook invocation with the default to call tree_invariant_p_1. After lowering we can reset the langhook to the default. I think we at least need the generic save_expr to be langhook aware during the time we fold the original non-gimplified GENERIC IL (but no longer for the generated GENERIC and to be gimplified IL the middle-end eventually generates later). Richard.