[PATCH] D105907: [CallGraphSection] Add call graph section options and documentation

[Necip Fazil Yildiran via Phabricator via llvm-commits]

necipfazil added inline comments.


================
Comment at: clang/docs/CallGraphSection.rst:58
+
+A type identifier may be repeated in different entries. The id value 0 is
+reserved for unknown and used for indirect targets with unknown type.
----------------
morehouse wrote:
> Why would a type ID be repeated?
Current implementation [1] creates a call graph section per function comdat group, to which the assembly printer writes the call graph entries related to the function.  This is done to enable dead-stripping of call graph entries.  Consequently, the callsites from two functions may share the same type ID but appear as distinct entries as they will be written to distinct sections.  Although they are merged to a single section by the linker, the type ID repetition persists since the linker only concatenates.

Eliminating this to ensure that type IDs are not repeated should only decrease the binary size overhead.

[1] https://reviews.llvm.org/D105916 , see MCObjectFileInfo::getCallGraphSection()


================
Comment at: clang/docs/CallGraphSection.rst:148
+  "foo", "int (char, void*)", "_ZTSFicPvE.generalized", "e3804d2a7f2b03fe"
+  "main", "int ()", "_ZTSFiE.generalized", "fa6809609a76afca"
+
----------------
morehouse wrote:
> Why quotes around these table headers/entries?
Quotes are placed to espace commas (actually, there is only one) in the table cells.  They are not visible in the output html.


================
Comment at: clang/docs/CallGraphSection.rst:160
+Notice that the current implementation may have seperate entries with the same
+type id as above.
+
----------------
morehouse wrote:
> Why is this?
Please see the response above.


================
Comment at: clang/docs/CallGraphSection.rst:174
+    fa6809609a76afca 401130
+    e3804d2a7f2b03fe 401110
+
----------------
morehouse wrote:
> Do we need to list functions that don't match any callsites (e.g., `main`)?
They may be needed for dynamically loaded objects whose functions are called indirectly outside the object.


================
Comment at: clang/docs/CallGraphSection.rst:183
+    401130 40115b
+    401170 4011b5
+
----------------
morehouse wrote:
> So this section is useful for constructing the call graph, but we don't really need it for stack trace reconstruction, right?
For stack trace reconstruction, we need the full reverse call graph, thus, this section. Without this section, which function makes the indirect call is unknown.

During stack trace reconstruction, let's say we end up in a state where an indirect call is seen. To take another step, we need to know which function made this indirect call, so that we can continue the search through the callers of that function.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105907