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

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added a comment.

High-level comments:

I'd use a bottom-up patch order. llvm patches are before clang patches.
The clang driver patch is the last - having the user-facing option requires the functionality to be fully available.
llvm-objdump patch is orthogonal to clang patches, so its order is flexible.

If the section only contains indirect call edges, perhaps it should have `indirect` in the option name.



================
Comment at: clang/docs/CallGraphSection.rst:9
+With ``-fcall-graph-section``, the compiler will create a call graph section 
+in the binary. It will include type identifiers for indirect calls and 
+targets. This information can be used to map indirect calls to their receivers 
----------------
object file


================
Comment at: clang/docs/CallGraphSection.rst:20
+any function in the program. This approach ensures completeness since no
+indirect call edge is missed. However, it is generally poor in precision
+due to having a much bigger than actual call graph with infeasible indirect
----------------
missing


================
Comment at: clang/docs/CallGraphSection.rst:21
+indirect call edge is missed. However, it is generally poor in precision
+due to having a much bigger than actual call graph with infeasible indirect
+call edges.
----------------
due to having unneeded edges 


================
Comment at: clang/docs/CallGraphSection.rst:29
+
+The ``llvm-objdump`` utility provides a ``-call-graph-info`` option to extract
+full call graph information by parsing the content of the call graph section
----------------
``--call-graph-info``


================
Comment at: clang/include/clang/Basic/CodeGenOptions.def:423
+/// Whether to emit a call graph section into the object file.
+CODEGENOPT(CallGraphSection, 1, 0)
+
----------------
make sense to move it near EmitCallSiteInfo


================
Comment at: clang/include/clang/Driver/Options.td:1258
   BothFlags<[CoreOption], " an address-significance table">>;
+defm call_graph_section : BoolFOption<"call-graph-section",
+  CodeGenOpts<"CallGraphSection">, DefaultFalse,
----------------
make sense to move it near stacksizessection


================
Comment at: clang/lib/CodeGen/BackendUtil.cpp:571
   Options.EmitAddrsig = CodeGenOpts.Addrsig;
+  Options.EmitCallGraphSection = CodeGenOpts.CallGraphSection;
   Options.ForceDwarfFrameSection = CodeGenOpts.ForceDwarfFrameSection;
----------------
make sense to move it near EmitCallSiteInfo


================
Comment at: clang/lib/Driver/ToolChains/Clang.cpp:6416
 
+  if (Args.hasFlag(options::OPT_fcall_graph_section,
+                   options::OPT_fno_call_graph_section, false))
----------------
make sense to move it near stacksizessection


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105907