Re: [PATCH] tree: Fix up save_expr [PR52339]

2023-05-30 Thread Jason Merrill via Gcc-patches
On 5/30/23 16:51, Jakub Jelinek wrote: On Tue, May 30, 2023 at 04:36:34PM -0400, Jason Merrill wrote: Note that it is fine to treat p->fld as invariant in C++ if fld is TREE_READONLY and p is itself invariant. The implementation is allowed to assume that other code didn't destroy *p and create

Re: [PATCH] tree: Fix up save_expr [PR52339]

2023-05-30 Thread Jakub Jelinek via Gcc-patches
On Tue, May 30, 2023 at 04:36:34PM -0400, Jason Merrill wrote: > Note that it is fine to treat p->fld as invariant in C++ if fld is > TREE_READONLY and p is itself invariant. The implementation is allowed to > assume that other code didn't destroy *p and create a new object with a > different valu

Re: [PATCH] tree: Fix up save_expr [PR52339]

2023-05-30 Thread Jason Merrill via Gcc-patches
On 5/30/23 04:23, Jakub Jelinek wrote: On Tue, May 30, 2023 at 10:03:05AM +0200, Eric Botcazou wrote: We want to be able to treat such things as invariant somehow even if we can't do that for references to user data that might be changed by intervening code. That is, indicate that we know that

Re: [PATCH] tree: Fix up save_expr [PR52339]

2023-05-30 Thread Jakub Jelinek via Gcc-patches
On Tue, May 30, 2023 at 10:03:05AM +0200, Eric Botcazou wrote: > > We want to be able to treat such things as invariant somehow even if we > > can't do that for references to user data that might be changed by > > intervening code. > > > > That is, indicate that we know that the _REF actually refe

Re: [PATCH] tree: Fix up save_expr [PR52339]

2023-05-30 Thread Eric Botcazou via Gcc-patches
> We want to be able to treat such things as invariant somehow even if we > can't do that for references to user data that might be changed by > intervening code. > > That is, indicate that we know that the _REF actually refers to a const > variable or is otherwise known to be unchanging. > > Per

Re: [PATCH] tree: Fix up save_expr [PR52339]

2023-05-28 Thread Jason Merrill via Gcc-patches
On 5/13/23 06:58, Eric Botcazou wrote: 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. I have been investigating this for a few days and it's no small change for Ada and probably for other la

Re: [PATCH] tree: Fix up save_expr [PR52339]

2023-05-13 Thread Eric Botcazou via Gcc-patches
> 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. I have been investigating this for a few days and it's no small change for Ada and probably for other languages with dynamic types. SAVE_E

Re: [PATCH] tree: Fix up save_expr [PR52339]

2023-05-09 Thread Eric Botcazou via Gcc-patches
> 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. DECL_INVARIANT_P is only set on discriminants with no default value and those are really invariant in Ada, i.e. do not change once set. >

Re: [PATCH] tree: Fix up save_expr [PR52339]

2023-05-08 Thread Jakub Jelinek via Gcc-patches
On Mon, May 08, 2023 at 06:23:54AM +, Richard Biener wrote: > 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. We cer

Re: [PATCH] tree: Fix up save_expr [PR52339]

2023-05-07 Thread Richard Biener via Gcc-patches
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.jj2023-01-16 23:19:05.539727388 > > > +0100 > > > +++ gcc/ada/gcc-interface/utils2.cc 2023-05-05 15:37:30.193990948 > > > +0200 > > > @@ -

Re: [PATCH] tree: Fix up save_expr [PR52339]

2023-05-05 Thread Jakub Jelinek via Gcc-patches
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

Re: [PATCH] tree: Fix up save_expr [PR52339]

2023-05-05 Thread Jason Merrill via Gcc-patches
On 5/5/23 09:40, Jakub Jelinek wrote: On Fri, May 05, 2023 at 07:38:45AM -0400, Jason Merrill wrote: On 5/5/23 06:45, Jakub Jelinek wrote: + if (TREE_READONLY (t) && !TREE_SIDE_EFFECTS (t)) +{ + /* Return true for const qualified vars, but for members or array +elements withou

Re: [PATCH] tree: Fix up save_expr [PR52339]

2023-05-05 Thread Jakub Jelinek via Gcc-patches
On Fri, May 05, 2023 at 07:38:45AM -0400, Jason Merrill wrote: > On 5/5/23 06:45, Jakub Jelinek wrote: > > + if (TREE_READONLY (t) && !TREE_SIDE_EFFECTS (t)) > > +{ > > + /* Return true for const qualified vars, but for members or array > > +elements without side-effects return true o

Re: [PATCH] tree: Fix up save_expr [PR52339]

2023-05-05 Thread Jason Merrill via Gcc-patches
On 5/5/23 06:45, Jakub Jelinek wrote: On Fri, May 05, 2023 at 11:55:41AM +0200, Jakub Jelinek via Gcc-patches wrote: Looking at the Ada cases (I admit I don't really understand why it isn't vectorized, the IL is so different from the start because of the extra SAVE_EXPRs that it is very hard to

Re: [PATCH] tree: Fix up save_expr [PR52339]

2023-05-05 Thread Jakub Jelinek via Gcc-patches
On Fri, May 05, 2023 at 11:55:41AM +0200, Jakub Jelinek via Gcc-patches wrote: > Looking at the Ada cases (I admit I don't really understand why it isn't > vectorized, the IL is so different from the start because of the extra > SAVE_EXPRs that it is very hard to diff stuff), the case where save_ex

Re: [PATCH] tree: Fix up save_expr [PR52339]

2023-05-05 Thread Jakub Jelinek via Gcc-patches
On Fri, May 05, 2023 at 11:04:09AM +0200, Jakub Jelinek via Gcc-patches wrote: > As mentioned in the PR, save_expr seems to be very optimistic when > some expression is invariant, which can result in various wrong-code > issues. > The problem is with the TREE_READONLY (t) && !TREE_SIDE_EFFECTS (t)