[PATCH] Fix up --enable-checking=fold (PR middle-end/89503)

Richard Biener rguenther@suse.de
Fri Mar 1 09:18:00 GMT 2019


On Fri, 1 Mar 2019, Jakub Jelinek wrote:

> On Fri, Mar 01, 2019 at 09:49:03AM +0100, Richard Biener wrote:
> > On Fri, 1 Mar 2019, Jakub Jelinek wrote:
> > > Some of the builtin folding sets TREE_NO_WARNING flags, which triggers as
> > > --enable-checking=fold errors.  I think we should allow setting of
> > > TREE_NO_WARNING flag when folding, so this patch arranges that.
> > > 
> > > Tested on:
> > > ../configure --enable-languages=c,c++,fortran --enable-checking=fold --disable-bootstrap
> > > make -jN && make -jN -k check
> > > No extra FAILs compared to normal regtest.  Ok for trunk?
> > 
> > Hmm, I guess it's OK though for EXPR_P I think we may want the
> > TREE_NO_WARNING setter to unshare the tree?  Because IIRC it is
> > exactly because of tree sharing that we do this kind of checking?
> 
> Maybe, I just wonder if it wouldn't be too expensive.
> These TREE_NO_WARNING sets are typically on arguments of builtin functions
> so we'd need to unshare the argument (shallow copy) as well as the whole
> builtin.  Some spots (usually older) in builtins.c do create new trees,
> but some TREE_NO_WARNING setting is hidden even in APIs like c_strlen
> which actually doesn't return the argument.  Plus, many of those functions
> that set it are used both during expansion (where supposedly we don't care
> about modifying in place) and folding.  What a mess?

Uh.

> So, do you prefer to drop this patch, or just take the  || EXPR_P (expr)
> part away from it and let Martin (who has added pretty much all of these
> TREE_NO_WARNING) fix it later?  c_strlen is probably the oldest with this
> issue and most likely very hard to change; the thing is, it is usually used
> during expansion and we heavily rely in that case that it modifies it
> in-place and thus doesn't warn multiple times.  Just use it also in
> fold_builtin_strlen :(.

No, as said the patch is probably OK (setting TREE_NO_WARNING is harmless
from a tree-sharing perspective but still undesired IMHO).

Maybe you can add a comment reflecting the above and open a bug so we
can eventually revisit this in future.

Thanks,
Richard.

> > > 2019-03-01  Jakub Jelinek  <jakub@redhat.com>
> > > 
> > > 	PR middle-end/89503
> > > 	* fold-const.c (fold_checksum_tree): Ignore TREE_NO_WARNING bit
> > > 	on DECL_P and EXPR_P.
> 
> 	Jakub



More information about the Gcc-patches mailing list