On 09/08/2024 17:56, Jakub Jelinek wrote:
> On Fri, Aug 09, 2024 at 04:46:55PM +0100, Alex Coplan wrote:
> > On 09/08/2024 17:34, Jakub Jelinek wrote:
> > > On Fri, Aug 09, 2024 at 04:21:14PM +0100, Alex Coplan wrote:
> > > > Hmm, good spot, I didn't realise that convert_from_reference could
> > > > change the type of the condition like this.
> > > >
> > > > In that case I suppose the only thing to do is to construct a stack of
> > > > ANNOTATE_EXPRs on the way down and re-build the expressions (taking the
> > > > type of the inner expression, starting with the cond) on the way back
> > > > up.
> > >
> > > I think you don't need to rebuild them, just update their types.
> > > Something along the lines of
> > > for (tree c = cond; c != *condp; c = TREE_OPERAND (c, 0))
> > > {
> > > gcc_checking_assert (TREE_CODE (c) == ANNOTATE_EXPR);
> > > TREE_TYPE (c) = TREE_TYPE (*condp);
> > > }
> >
> > I suppose I was just concerned about any other properties of
> > ANNOTATE_EXPRs that might be inherited from the operand (that could be
> > affected by such a change).
> >
> > It looks like TREE_{READONLY,THIS_VOLATILE,SIDE_EFFECTS} are all set
> > in convert_from_reference and propagated by build3, so if those change
> > underneath us then only updating the type seems insufficient.
>
> I think TREE_THIS_VOLATILE isn't, that is only for references.
> The others could change, but only in the !TREE_SIDE_EFFECTS ->
> TREE_SIDE_EFFECTS or TREE_READONLY -> !TREE_READONLY direction
> (the former if it was volatile bool &).
> So you could also in the loop update it just in case,
> TREE_SIDE_EFFECTS (c) |= TREE_SIDE_EFFECTS (*condp);
> TREE_READONLY (c) &= TREE_READONLY (*condp);
Any reason not to just update those unconditionally? I.e. just make
those normal assignments? I'm assuming we'll only run the loop at all
in the case that TREE_TYPE (*condp) != TREE_TYPE (cond).
Alex
>
> Jakub
>
