On Fri, 29 Apr 2022, Jakub Jelinek wrote: > On Fri, Apr 29, 2022 at 11:52:37AM +0200, Richard Biener wrote: > > On Fri, 29 Apr 2022, Jakub Jelinek wrote: > > > > > On Fri, Apr 29, 2022 at 11:32:15AM +0200, Richard Biener wrote: > > > > I think that's reasonable (we indeed shouldn't create a varpool node > > > > here). I do think that we eventually want to retain removed nodes > > > > but mark them so. In fact any debug references will be thrown away > > > > because of this anyway. > > > > > > > > So I wonder if we can instead simply do if (!x_node) return 0;? > > > > > > I had that in my first version, but after finding out that it triggers > > > so often for the constant pool decls I thought better to just use > > > x_decl in that case instead of x_node->decl. > > > I must say I'm unsure if constant pool decls always stay out of section > > > anchors or if they can be put there too. > > > > > > > The question is also why sched does any queries for debug-insns, > > > > does it merely reset them based on the answer? That said, > > > > it would be nice to be able to assert that x_node is not NULL > > > > and catch this in the callers somehow. > > > > > > Unfortunately, several layers of callers don't really know it is for debug > > > insn. And the touched code is solely for the section anchors, so e.g. > > > just > > > checking symtab_node::get (decl) on all mentioned decls when we perhaps > > > can > > > see if it is debug insn or not would be quite costly and we wouldn't know > > > if the other reference is anchored. > > > > We might want to reset debug stmts at the time we RTL expand them > > if referred symbols have no cgraph node? As said, ->get () instead > > of ->get_create () is obviously OK but the way we deal with the fallout > > is a bit suspicious there IMHO. > > So, what about doing that if (!x_node) return 0; in alias.c with the > exception of DECL_IN_CONSTANT_POOL, plus in cfgexpand.cc throw away > VAR_DECLs without symtab node?
So we don't have varpool nodes for the constant pool decls? Are they CONST_DECLs at least? I think that's reasonable though ideally we'd be able to assert that there is a symtab node for the decls ... > I'll need to do some extra checking on whether we don't really lose any > useful debug info with the second patch. At least it was surprisingly simple ;) Richard.
