On Thu, 4 May 2017, Jakub Jelinek wrote: > On Thu, May 04, 2017 at 06:21:17PM +0200, Richard Biener wrote: > > >the > > >only other user after all calls free_node in a loop, so it is highly > > >unlikely it would do anything there. > > > > > >If you mean the INTEGER_TYPE handling, then yes, I guess it could be > > >done in free_node too and can move it there. If it was without > > >the && TREE_TYPE (TYPE_M*_VALUE (type)) == type extra checks, then it > > >is certainly unsafe and breaks bootstrap even, e.g. build_range_type > > >and other spots happily create INTEGER_TYPEs with min/max value that > > >have some other type. But when the type of the INTEGER_CSTs is the > > >type we are ggc_freeing, anything that would refer to those constants > > >afterwards would be necessarily broken (as their TREE_TYPE would be > > >ggc_freed, possibly reused for something completely unrelated). > > >Thus I think it should be safe even in the LTO case and thus doable > > >in free_node. > > > > OK. OTOH LTO frees the whole SCC and thus doesn't expect any pointed to > > stuff > > to be freed. Not sure if we allow double ggc_free of stuff. > > We don't, that crashes miserably.
Ok, so then keep it in type_hash_canon (with a comment). Otherwise the patch is ok. Thanks, Richard.