Re: [PATCH] alias: Fix -fcompare-debug issues caused by compare_base_symbol_refs [PR105415]

[Richard Biener <rguenther@suse.de>]

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.