[PATCH] D101025: [llvm-dwarfdump] Fix inline function stats calculation

[Djordje Todorovic via Phabricator via llvm-commits]

djtodoro added inline comments.


================
Comment at: llvm/tools/llvm-dwarfdump/Statistics.cpp:470-472
+  const bool IsInlinedFunction =
+      (Tag == dwarf::DW_TAG_inlined_subroutine ||
+       (Tag == dwarf::DW_TAG_subprogram && HasAbstractOriginAttr));
----------------
dblaikie wrote:
> djtodoro wrote:
> > dblaikie wrote:
> > > This name and the associated names (InlineFnsToBeProcessed, etc) may be incorrect now?
> > > 
> > > (or it looks like, if we're tracking "InlinedVars" Separately, perhaps this variable is the wrong one for tracking those - yeah, judging by the statistics that have changed - we probably shouldn't be grouping these non-inline concrete definitions under the "#inlined functions" or "#inlined functions with abstract origins" (also not sure why we have those two different statistics - inlined functions without abstract origins are non-DWARF-conforming, I think)
> > Hmmm.... I am a bit confused. :)
> > 
> > If we have a concrete `DW_TAG_subprogram` with an `abstract_origin` (representing e.g. an `always_inline` function), are we counting it as an inlined function or as a concrete one?
> > 
> > If it is a concrete function, we should rename `InlineFnsToBeProcessed`, etc.
> I believe we should be counting it as a concrete one - are we not?
> 
> Looks like we are counting them as concrete.
> 
> Given this example:
> ```
> __attribute__((nodebug)) void f1();
> void f2() { f1(); }
> void f3() { f2(); }
> ```
> (compiled with -O3 to force the inlining, or use attributes to do the same)
> Produces DWARF something like this:
> ```
> 0x0000000b: DW_TAG_compile_unit
>               DW_AT_name        ("test.cpp")
> 0x0000002a:   DW_TAG_subprogram
>                 DW_AT_abstract_origin   (0x0000003d "_Z2f2v")
> 0x0000003d:   DW_TAG_subprogram
>                 DW_AT_name      ("f2")
> 0x00000049:   DW_TAG_subprogram
>                 DW_AT_name      ("f3")
> 0x00000062:     DW_TAG_inlined_subroutine
>                   DW_AT_abstract_origin (0x0000003d "_Z2f2v")
> ```
> The relevant stats are:
> ```
>   "#functions": 2,
>   "#functions with location": 2,
>   "#inlined functions": 1,
>   "#inlined functions with abstract origins": 1,
> ```
> (I don't know why we have a separate stat for `#inlined functions` compared to `#inlined functions with abstract origins` - pretty sure the latter should just be a verifier error (& not one I'd expect to be common either) rather than a statistic)
> 
> So the two functions should me `f2` and `f3` (`0x0000002a` and `0x00000049`, but not `0x0000003d`).
> The inlined function is the inlined version of `f2` (`0x00000062`)
> 
> 
> I believe we should be counting it as a concrete one - are we not?
> 
Actually yes.

> Looks like we are counting them as concrete.
> 
> Given this example:
> ```
> __attribute__((nodebug)) void f1();
> void f2() { f1(); }
> void f3() { f2(); }
> ```
> (compiled with -O3 to force the inlining, or use attributes to do the same)
> Produces DWARF something like this:
> ```
> 0x0000000b: DW_TAG_compile_unit
>               DW_AT_name        ("test.cpp")
> 0x0000002a:   DW_TAG_subprogram
>                 DW_AT_abstract_origin   (0x0000003d "_Z2f2v")
> 0x0000003d:   DW_TAG_subprogram
>                 DW_AT_name      ("f2")
> 0x00000049:   DW_TAG_subprogram
>                 DW_AT_name      ("f3")
> 0x00000062:     DW_TAG_inlined_subroutine
>                   DW_AT_abstract_origin (0x0000003d "_Z2f2v")
> ```
> The relevant stats are:
> ```
>   "#functions": 2,
>   "#functions with location": 2,
>   "#inlined functions": 1,
>   "#inlined functions with abstract origins": 1,
> ```
> (I don't know why we have a separate stat for `#inlined functions` compared to `#inlined functions with abstract origins` - pretty sure the latter should just be a verifier error (& not one I'd expect to be common either) rather than a statistic)
> 
Hmmm, I agree, there is no point of having `inlined_subroutine` with no `abstract_origin`, right?

> So the two functions should me `f2` and `f3` (`0x0000002a` and `0x00000049`, but not `0x0000003d`).
> The inlined function is the inlined version of `f2` (`0x00000062`)
> 




CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D101025/new/

https://reviews.llvm.org/D101025