llvm-dwarfdump stats for inlined functions

Hi,

As discussed on the https://reviews.llvm.org/D101025, we have noticed that there are two different stat categories for inlined functions when using llvm-dwarfdump –statistics:

inlined functions

inlined functions with abstract origin

and it was introduced after the D58849. We were wondering if there is a particular motivation of doing so.

Having an inlined_subroutine DIE with no abstract_origin attribute does not have so many benefits, so it might be better if we have an assertion/error (e.g., in the AsmPrinter) when we face such situation (or just to avoid dumping it into the final DWARF).

Any thoughts?

Best regards,

Djordje

Pedantically, DW_AT_abstract_origin is not required on a DW_TAG_inlined_subroutine; the DWARF spec says a concrete instance “may” omit attributes not specific to that instance, in which case there will be an abstract origin that will have those attributes. It then goes on to point out that declaration coordinates (DW_AT_decl_[file,line,column]) are for the declaration, not the inlining site, which makes them good candidates for hoisting to the abstract origin.

That is, DW_AT_abstract_origin is a way to optimize for size; it isn’t mandatory. So, I think llvm-dwarfdump is correct to allow it. I might prefer that “inlined functions without abstract origin” be what’s called out separately, rather than the “with” case, as the “without” cases represent a likely opportunity for size reduction.

I don’t think we ever emit an inlined_subroutine without an abstract origin? If we want to make it an assertion on the emission side, that would be fine. In fact, now that we omit DW_TAG_formal_parameter where the parameter has been optimized away, it’s probably a good idea to have an assertion like that.

–paulr

Hi,

As discussed on the https://reviews.llvm.org/D101025, we have noticed that there are two different stat categories for inlined functions when using llvm-dwarfdump –statistics:

inlined functions

inlined functions with abstract origin

and it was introduced after the D58849. We were wondering if there is a particular motivation of doing so.

When I wrote D58849 I wasn’t sure if all the inlined functions would have abstract origins or not; that was one of the things I wanted to check on.

– Caroline
cmtice@google.com

Hi Paul,

Thanks for your response.

I also think that “inlined function without abstract origin” could be better stats.

I don’t think we ever emit an inlined_subroutine without an abstract origin? If we want to make it an assertion on the emission side, that would be fine. In fact, now that we omit DW_TAG_formal_parameter where the parameter has been optimized away, it’s probably a good idea to have an assertion like that.

Sure, thanks. I am not sure either whether we emit such inlined_subroutine, but we can make an assertion within the AsmPrinter, and to see if there is a situation like that.

Best regards,
Djordje

Hi Caroline,

Thanks for your response.

When I wrote D58849 I wasn’t sure if all the inlined functions would have abstract origins or not; that was one of the things I wanted to check on.

OK, as Paul mentioned in the previous mail, it might be better if we calculated the “#inlined functions without abstract origin”.

Thanks,
Djordje

That is fine with me if you want to make that change.

Pedantically, DW_AT_abstract_origin is not required on a DW_TAG_inlined_subroutine; the DWARF spec says a concrete instance “may” omit attributes not specific to that instance, in which case there will be an abstract origin that will have those attributes. It then goes on to point out that declaration coordinates (DW_AT_decl_[file,line,column]) are for the declaration, not the inlining site, which makes them good candidates for hoisting to the abstract origin.

Sure enough. Potentially one could argue that abstract_origin is
necessary to describe the scope where the original entity was written
(that can't be/isn't described in attributes) - though using
DW_AT_specification to refer to a declaration in the source scope
would probably suffice (even if that wasn't exactly representative of
the source code - perhaps the declaration was in one place, the
definition in another, and the inlining in a third place - there
wouldn't be any way to describe that second place without
abstract_origin (but there's probbaly not a lot of value in describing
that second place anyway).

That is, DW_AT_abstract_origin is a way to optimize for size; it isn’t mandatory. So, I think llvm-dwarfdump is correct to allow it. I might prefer that “inlined functions without abstract origin” be what’s called out separately, rather than the “with” case, as the “without” cases represent a likely opportunity for size reduction.

I don’t *think* we ever emit an inlined_subroutine without an abstract origin? If we want to make it an assertion on the emission side, that would be fine. In fact, now that we omit DW_TAG_formal_parameter where the parameter has been optimized away, it’s probably a good idea to have an assertion like that.

I don't believe we have - and the architecture isn't really designed
for that as an option. If there's some good places for extra
assertions to ensure it (no doubt we've had bugs where we fail to emit
the abstract origin or stitch it up to the inlined or concrete
definition) that's OK - but I probably wouldn't go hunting for it or
personally wouldn't go tracking it as a statistic (at least for LLVM's
output, any time those two stats differ would be a bug - so I'd
probably track it more with a light warning (not hard error) in the
debug info verifier mode, perhaps - if we have warnings/if it is
important)

Hi Caroline,

Thanks for your response.

>When I wrote D58849 I wasn't sure if all the inlined functions would have abstract origins or not; that was one of the things I wanted to check on.

OK, as Paul mentioned in the previous mail, it might be better if we calculated the "#inlined functions without abstract origin".

If we're keeping the stat anyway, I suspect it's probably not worth
the hassle of renaming/reving the format/etc. But I don't have strong
opinions on it.

- Dave

> >When I wrote D58849 I wasn't sure if all the inlined functions would
have abstract origins or not; that was one of the things I wanted to check
on.
>
> OK, as Paul mentioned in the previous mail, it might be better if we
calculated the "#inlined functions without abstract origin".

If we're keeping the stat anyway, I suspect it's probably not worth
the hassle of renaming/reving the format/etc. But I don't have strong
opinions on it.

If there's a database somewhere, probably not worth messing with the
schema. For reporting to humans, I'd have a slight preference for
what I said, but it's not a big deal.
--paulr

Yep.

Also: Even though these cases can legitimately be emitted, I'm not
sure they're worth tracking the difference (. I'd still be marginally
in favor of removing the extra stat (removing the stat that tracks
"without abstract_origin" and renaming the other one). We don't track
"subprograms with DW_AT_specification" or various other properties,
for instance.