This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] Fix inlining of calls with NULL block (PR56515)


On Mon, 4 Mar 2013, Jan Hubicka wrote:

> > On Mon, Mar 04, 2013 at 01:42:51PM +0100, Richard Biener wrote:
> > > When inlining call stmts with a NULL gimple_block we still remap
> > > all the callee blocks into a block tree copy but we'll end up
> > > not referencing it from anywhere.  This causes verification failures
> > > because then we have nothing refering to the inline stmt blocks.
> > 
> > Ugh, best would be to set proper block even for the artificially added calls
> > (whether to look around for surrounding statement blocks or similar).
> 
> As I added to the PR log, I do not really think we can LTO reliably libraries,
> like libgcov, where backend invents its own calls.  For safe LTO of runtime
> bits (libgcc/libgcov/libc) we will need a mechanizm to explicitly mark 
> implementations of runtime calls and do not remove them from callgraph until
> it is clear that no new calls are invented.

Indeed.  Still the issue exists.

> > Because clearing the block tree will have the nasty effect that nothing
> > in the inlined routine will be debuggable.  Attaching the block tree to
> > DECL_INITIAL isn't very good either, because while the inline fn itself
> > will be debuggable, the parent function's variables won't be accessible.
> > 
> > But I guess your patch is fine as a hack to avoid ICEs, and we just should
> > try harder and harder to avoid ever hitting that situation.
> 
> Well, what would you recommend to do when adding an instrumentation calls on
> random places of the callgraph?
> We do
> 1) adding the counter increments that are associated with edges in callgraph.
> 2) adding VPT calls that are associated with an instruction (like divide)
> 3) adding calls to prologue handling indirect call.
> 
> I suppose 3) can be handled same way as prologue code and 2) can copy block
> from the associated instruction. But what should we do for 1?

Note that "copy block from surrounding code" is equivalent to
a NULL block (just inherit the currently active block).  What's
more interesting is whether we want debug information for the
inlined code at all - after all the callees are all DECL_ARTIFICIAL
and definitely middle-end introduced memcpy calls diving into
inlined memcpy implementation would be odd - after all the source
doesn't contain the memcpy call and so I think there is no way to
step "over" the call (after all we don't want to report it!).

Thus I think for inlined artificial calls we want to drop debug
information from its inlined body.  The patch does that for
BLOCKs (and thus variables - which effectively means everything
but line information).

What we eventually want to have is a BLOCK marked as
"inlined outer scope of artificial function FOO".  Not sure
if dwarf can express that though.  Then gdb could do the
right thing and other debug consumers like systemtap could
still see the implementation.

I've now LTO profilebootstrapped and bootstrapped and tested
the patch on x86_64-unknown-linux-gnu and plan to install it
with a big fat comment added.

Thanks,
Richard.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]