[PATCH] D105909: [clang][CallGraphSection] Add type id metadata to indirect call and targets

[Necip Fazil Yildiran via Phabricator via cfe-commits]

necipfazil added inline comments.



Comment at: clang/lib/CodeGen/CodeGenModule.cpp:2068
+EmittedMDIdGeneralized = true;
+  }
+

morehouse wrote:
> This code also seems unnecessary as it puts metadata on function definitions.
We need the metadata for mapping indirect calls to receiver indirect targets 
with matching type id.



Comment at: clang/lib/CodeGen/CodeGenModule.cpp:2177
+  !CodeGenOpts.SanitizeCfiCanonicalJumpTables ||
+  CodeGenOpts.CallGraphSection)
 CreateFunctionTypeMetadataForIcall(FD, F);

morehouse wrote:
> Also seems unnecessary.
As described above, we need the metadata for indirect targets.



Comment at: clang/test/CodeGen/call-graph-section.c:20
+
+// CHECK-DAG: define {{(dso_local)?}} i32 @baz({{.*}} !type 
[[F_TPRIMITIVE:![0-9]+]]
+int baz(char a, float b, double c) {

morehouse wrote:
> Do we still expect type metadata on function definitions on the latest diff?
As described above, we need the metadata for indirect targets.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105909

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D105909: [clang][CallGraphSection] Add type id metadata to indirect call and targets

[Necip Fazil Yildiran via Phabricator via cfe-commits]

necipfazil updated this revision to Diff 358996.
necipfazil added a comment.

- rebase
- add C struct parameter test
- add C++ tests


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105909

Files:
  clang/lib/CodeGen/CGClass.cpp
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/CodeGen/CGExprCXX.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/CodeGen/CodeGenModule.h
  clang/test/CodeGen/call-graph-section-1.cpp
  clang/test/CodeGen/call-graph-section-2.cpp
  clang/test/CodeGen/call-graph-section-3.cpp
  clang/test/CodeGen/call-graph-section.c

Index: clang/test/CodeGen/call-graph-section.c
===
--- /dev/null
+++ clang/test/CodeGen/call-graph-section.c
@@ -0,0 +1,87 @@
+// Tests that we assign appropriate identifiers to indirect calls and targets.
+
+// RUN: %clang_cc1 -triple x86_64-unknown-linux -fcall-graph-section -S \
+// RUN: -emit-llvm -o - %s | FileCheck --check-prefixes=CHECK,ITANIUM %s
+
+// RUN: %clang_cc1 -triple x86_64-pc-windows-msvc -fcall-graph-section -S \
+// RUN: -emit-llvm -o - %s | FileCheck --check-prefixes=CHECK,MS %s
+
+// CHECK-DAG: define {{(dso_local)?}} void @foo({{.*}} !type [[F_TVOID:![0-9]+]]
+void foo() {
+}
+
+// CHECK-DAG: define {{(dso_local)?}} void @bar({{.*}} !type [[F_TVOID]]
+void bar() {
+  void (*fp)() = foo;
+  // CHECK: call {{.*}} !type [[CS_TVOID:![0-9]+]]
+  fp();
+}
+
+// CHECK-DAG: define {{(dso_local)?}} i32 @baz({{.*}} !type [[F_TPRIMITIVE:![0-9]+]]
+int baz(char a, float b, double c) {
+  return 1;
+}
+
+// CHECK-DAG: define {{(dso_local)?}} i32* @qux({{.*}} !type [[F_TPTR:![0-9]+]]
+int *qux(char *a, float *b, double *c) {
+  return 0;
+}
+
+// CHECK-DAG: define {{(dso_local)?}} void @corge({{.*}} !type [[F_TVOID]]
+void corge() {
+  int (*fp_baz)(char, float, double) = baz;
+  // CHECK: call i32 {{.*}}, !type [[CS_TPRIMITIVE:![0-9]+]]
+  fp_baz('a', .0f, .0);
+
+  int *(*fp_qux)(char *, float *, double *) = qux;
+  // CHECK: call i32* {{.*}}, !type [[CS_TPTR:![0-9]+]]
+  fp_qux(0, 0, 0);
+}
+
+struct st1 {
+  int *(*fp)(char *, float *, double *);
+};
+
+struct st2 {
+  struct st1 m;
+};
+
+// CHECK-DAG: define {{(dso_local)?}} void @stparam({{.*}} !type [[F_TSTRUCT:![0-9]+]]
+void stparam(struct st2 a, struct st2 *b) {}
+
+// CHECK-DAG: define {{(dso_local)?}} void @stf({{.*}} !type [[F_TVOID]]
+void stf() {
+  struct st1 St1;
+  St1.fp = qux;
+  // CHECK: call i32* {{.*}}, !type [[CS_TPTR]]
+  St1.fp(0, 0, 0);
+
+  struct st2 St2;
+  St2.m.fp = qux;
+  // CHECK: call i32* {{.*}}, !type [[CS_TPTR]]
+  St2.m.fp(0, 0, 0);
+
+  // CHECK: call void {{.*}} !type [[CS_TSTRUCT:![0-9]+]]
+  void (*fp_stparam)(struct st2, struct st2 *) = stparam;
+  fp_stparam(St2, &St2);
+}
+
+// ITANIUM-DAG: [[F_TVOID]] = !{i64 0, !"_ZTSFvE.generalized"}
+// ITANIUM-DAG: [[CS_TVOID]] = !{!"_ZTSFvE.generalized"}
+// MS-DAG: [[F_TVOID]] = !{i64 0, !"?6AX@Z.generalized"}
+// MS-DAG: [[CS_TVOID]] = !{!"?6AX@Z.generalized"}
+
+// ITANIUM-DAG: [[F_TPRIMITIVE]] = !{i64 0, !"_ZTSFicfdE.generalized"}
+// ITANIUM-DAG: [[CS_TPRIMITIVE]] = !{!"_ZTSFicfdE.generalized"}
+// MS-DAG: [[F_TPRIMITIVE]] = !{i64 0, !"?6AHDMN@Z.generalized"}
+// MS-DAG: [[CS_TPRIMITIVE]] = !{!"?6AHDMN@Z.generalized"}
+
+// ITANIUM-DAG: [[F_TPTR]] = !{i64 0, !"_ZTSFPvS_S_S_E.generalized"}
+// ITANIUM-DAG: [[CS_TPTR]] = !{!"_ZTSFPvS_S_S_E.generalized"}
+// MS-DAG: [[F_TPTR]] = !{i64 0, !"?6APEAXPEAX00@Z.generalized"}
+// MS-DAG: [[CS_TPTR]] = !{!"?6APEAXPEAX00@Z.generalized"}
+
+// ITANIUM-DAG: [[F_TSTRUCT]] = !{i64 0, !"_ZTSFv3st2PvE.generalized"}
+// MS-DAG: [[F_TSTRUCT]] = !{i64 0, !"?6AXUst2@@PEAX@Z.generalized"}
+// ITANIUM-DAG: [[CS_TSTRUCT]] = !{!"_ZTSFv3st2PvE.generalized"}
+// MS-DAG: [[CS_TSTRUCT]] = !{!"?6AXUst2@@PEAX@Z.generalized"}
\ No newline at end of file
Index: clang/test/CodeGen/call-graph-section-3.cpp
===
--- /dev/null
+++ clang/test/CodeGen/call-graph-section-3.cpp
@@ -0,0 +1,54 @@
+// Tests that we assign appropriate identifiers to indirect calls and targets
+// specifically for virtual methods.
+
+// RUN: %clang_cc1 -triple x86_64-unknown-linux -fcall-graph-section -S \
+// RUN: -emit-llvm -o %t %s
+// RUN: FileCheck --check-prefix=FT %s < %t
+// RUN: FileCheck --check-prefix=CST %s < %t
+
+
+// Class definitions (check for indirect target metadata)
+
+class Base {
+public:
+  // FT-DAG: define {{.*}} @_ZN4Base2vfEPc({{.*}} !type [[F_TVF:![0-9]+]]
+  virtual int vf(char *a) { return 0; };
+};
+
+class Derived : public Base {
+public:
+  // FT-DAG: define {{.*}} @_ZN7Derived2vfEPc({{.*}} !type [[F_TVF]]
+  int vf(char *a) override { return 1; };
+};
+
+// FT-DAG: [[F_TVF]] = !{i64 0, !"_ZTSFiPvE.generalized"}
+
+
+// Callsites (c

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

[Necip Fazil Yildiran via Phabricator via cfe-commits]

necipfazil updated this revision to Diff 359010.
necipfazil marked 2 inline comments as done.
necipfazil added a comment.

- test new clang flags
- rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105907

Files:
  clang/docs/CallGraphSection.rst
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/clang_f_opts.c
  llvm/include/llvm/CodeGen/CommandFlags.h
  llvm/include/llvm/Target/TargetOptions.h
  llvm/lib/CodeGen/CommandFlags.cpp

Index: llvm/lib/CodeGen/CommandFlags.cpp
===
--- llvm/lib/CodeGen/CommandFlags.cpp
+++ llvm/lib/CodeGen/CommandFlags.cpp
@@ -86,6 +86,7 @@
 CGOPT(DebuggerKind, DebuggerTuningOpt)
 CGOPT(bool, EnableStackSizeSection)
 CGOPT(bool, EnableAddrsig)
+CGOPT(bool, EnableCallGraphSection)
 CGOPT(bool, EmitCallSiteInfo)
 CGOPT(bool, EnableMachineFunctionSplitter)
 CGOPT(bool, EnableDebugEntryValues)
@@ -407,6 +408,11 @@
   cl::init(false));
   CGBINDOPT(EnableAddrsig);
 
+  static cl::opt EnableCallGraphSection(
+  "call-graph-section", cl::desc("Emit a call graph section"),
+  cl::init(false));
+  CGBINDOPT(EnableCallGraphSection);
+
   static cl::opt EmitCallSiteInfo(
   "emit-call-site-info",
   cl::desc(
@@ -520,6 +526,7 @@
   Options.EmitStackSizeSection = getEnableStackSizeSection();
   Options.EnableMachineFunctionSplitter = getEnableMachineFunctionSplitter();
   Options.EmitAddrsig = getEnableAddrsig();
+  Options.EmitCallGraphSection = getEnableCallGraphSection();
   Options.EmitCallSiteInfo = getEmitCallSiteInfo();
   Options.EnableDebugEntryValues = getEnableDebugEntryValues();
   Options.PseudoProbeForProfiling = getPseudoProbeForProfiling();
Index: llvm/include/llvm/Target/TargetOptions.h
===
--- llvm/include/llvm/Target/TargetOptions.h
+++ llvm/include/llvm/Target/TargetOptions.h
@@ -127,11 +127,11 @@
   EmulatedTLS(false), ExplicitEmulatedTLS(false), EnableIPRA(false),
   EmitStackSizeSection(false), EnableMachineOutliner(false),
   EnableMachineFunctionSplitter(false), SupportsDefaultOutlining(false),
-  EmitAddrsig(false), EmitCallSiteInfo(false),
-  SupportsDebugEntryValues(false), EnableDebugEntryValues(false),
-  PseudoProbeForProfiling(false), ValueTrackingVariableLocations(false),
-  ForceDwarfFrameSection(false), XRayOmitFunctionIndex(false),
-  DebugStrictDwarf(false),
+  EmitAddrsig(false), EmitCallGraphSection(false),
+  EmitCallSiteInfo(false), SupportsDebugEntryValues(false),
+  EnableDebugEntryValues(false), PseudoProbeForProfiling(false),
+  ValueTrackingVariableLocations(false), ForceDwarfFrameSection(false),
+  XRayOmitFunctionIndex(false), DebugStrictDwarf(false),
   FPDenormalMode(DenormalMode::IEEE, DenormalMode::IEEE) {}
 
 /// DisableFramePointerElim - This returns true if frame pointer elimination
@@ -290,6 +290,9 @@
 /// to selectively generate basic block sections.
 std::shared_ptr BBSectionsFuncListBuf;
 
+/// Emit section containing call graph metadata.
+unsigned EmitCallGraphSection : 1;
+
 /// The flag enables call site info production. It is used only for debug
 /// info, and it is restricted only to optimized code. This can be used for
 /// something else, so that should be controlled in the frontend.
Index: llvm/include/llvm/CodeGen/CommandFlags.h
===
--- llvm/include/llvm/CodeGen/CommandFlags.h
+++ llvm/include/llvm/CodeGen/CommandFlags.h
@@ -122,6 +122,8 @@
 
 bool getEnableAddrsig();
 
+bool getEnableCallGraphSection();
+
 bool getEmitCallSiteInfo();
 
 bool getEnableMachineFunctionSplitter();
Index: clang/test/Driver/clang_f_opts.c
===
--- clang/test/Driver/clang_f_opts.c
+++ clang/test/Driver/clang_f_opts.c
@@ -231,6 +231,7 @@
 // RUN: -falign-functions -falign-functions=2 -fno-align-functions\
 // RUN: -fasynchronous-unwind-tables -fno-asynchronous-unwind-tables  \
 // RUN: -fbuiltin -fno-builtin\
+// RUN: -fcall-graph-section -fno-call-graph-section  \
 // RUN: -fdiagnostics-show-location=once  \
 // RUN: -ffloat-store -fno-float-store\
 // RUN: -feliminate-unused-debug-types -fno-eliminate-unused-debug-types  \
@@ -596,3 +597,9 @@
 // RUN: %clang -### -xobjective-c %s 2>&1 | FileCheck -check-prefix=CHECK_NO_DISABLE_DIRECT %s
 // CHECK_DISABLE_DIRECT: -fobjc-disable-direct-methods-for-testing
 // CHECK_NO_DISABLE_DIRECT-NOT: -fobjc-disa

[PATCH] D105909: [clang][CallGraphSection] Add type id metadata to indirect call and targets

[Necip Fazil Yildiran via Phabricator via cfe-commits]

necipfazil updated this revision to Diff 359401.
necipfazil added a comment.
Herald added subscribers: dexonsmith, hiraditya.
Herald added a project: LLVM.

Use operand bundles for callsite type ids

- Use (unlossy) operand bundles for propagating indirect callsite type ids
- No longer use type metadata for indirect callsites
- Adapt the tests to the changes

Currently, all callsites are annotated with a `type` operand bundle regardless
of whether indirect or not. While this does not affect the correctness for
call graph section as the extra information is not used, unnecessary operand
bundles on direct callsites increases IR program size. This will be fixed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105909

Files:
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/test/CodeGen/call-graph-section-1.cpp
  clang/test/CodeGen/call-graph-section-2.cpp
  clang/test/CodeGen/call-graph-section-3.cpp
  clang/test/CodeGen/call-graph-section.c
  llvm/include/llvm/IR/LLVMContext.h
  llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp

Index: llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
===
--- llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
+++ llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
@@ -2877,7 +2877,7 @@
  {LLVMContext::OB_deopt, LLVMContext::OB_gc_transition,
   LLVMContext::OB_gc_live, LLVMContext::OB_funclet,
   LLVMContext::OB_cfguardtarget,
-  LLVMContext::OB_clang_arc_attachedcall}) &&
+  LLVMContext::OB_clang_arc_attachedcall, LLVMContext::OB_type}) &&
  "Cannot lower invokes with arbitrary operand bundles yet!");
 
   const Value *Callee(I.getCalledOperand());
@@ -2960,8 +2960,9 @@
 
   // Deopt bundles are lowered in LowerCallSiteWithDeoptBundle, and we don't
   // have to do anything here to lower funclet bundles.
-  assert(!I.hasOperandBundlesOtherThan(
- {LLVMContext::OB_deopt, LLVMContext::OB_funclet}) &&
+  assert(!I.hasOperandBundlesOtherThan({LLVMContext::OB_deopt,
+LLVMContext::OB_funclet,
+LLVMContext::OB_type}) &&
  "Cannot lower callbrs with arbitrary operand bundles yet!");
 
   assert(I.isInlineAsm() && "Only know how to handle inlineasm callbr");
@@ -8086,7 +8087,7 @@
   assert(!I.hasOperandBundlesOtherThan(
  {LLVMContext::OB_deopt, LLVMContext::OB_funclet,
   LLVMContext::OB_cfguardtarget, LLVMContext::OB_preallocated,
-  LLVMContext::OB_clang_arc_attachedcall}) &&
+  LLVMContext::OB_clang_arc_attachedcall, LLVMContext::OB_type}) &&
  "Cannot lower calls with arbitrary operand bundles!");
 
   SDValue Callee = getValue(I.getCalledOperand());
Index: llvm/include/llvm/IR/LLVMContext.h
===
--- llvm/include/llvm/IR/LLVMContext.h
+++ llvm/include/llvm/IR/LLVMContext.h
@@ -94,6 +94,7 @@
 OB_preallocated = 4,   // "preallocated"
 OB_gc_live = 5,// "gc-live"
 OB_clang_arc_attachedcall = 6, // "clang.arc.attachedcall"
+OB_type = 7,   // "type"
   };
 
   /// getMDKindID - Return a unique non-zero ID for the specified metadata kind.
Index: clang/test/CodeGen/call-graph-section.c
===
--- /dev/null
+++ clang/test/CodeGen/call-graph-section.c
@@ -0,0 +1,85 @@
+// Tests that we assign appropriate identifiers to indirect calls and targets.
+
+// RUN: %clang_cc1 -triple x86_64-unknown-linux -fcall-graph-section -S \
+// RUN: -emit-llvm -o - %s | FileCheck --check-prefixes=CHECK,ITANIUM %s
+
+// RUN: %clang_cc1 -triple x86_64-pc-windows-msvc -fcall-graph-section -S \
+// RUN: -emit-llvm -o - %s | FileCheck --check-prefixes=CHECK,MS %s
+
+// CHECK-DAG: define {{(dso_local)?}} void @foo({{.*}} !type [[F_TVOID:![0-9]+]]
+void foo() {
+}
+
+// CHECK-DAG: define {{(dso_local)?}} void @bar({{.*}} !type [[F_TVOID]]
+void bar() {
+  void (*fp)() = foo;
+  // ITANIUM: call {{.*}} [ "type"(metadata !"_ZTSFvE.generalized") ]
+  // MS:  call {{.*}} [ "type"(metadata !"?6AX@Z.generalized") ]
+  fp();
+}
+
+// CHECK-DAG: define {{(dso_local)?}} i32 @baz({{.*}} !type [[F_TPRIMITIVE:![0-9]+]]
+int baz(char a, float b, double c) {
+  return 1;
+}
+
+// CHECK-DAG: define {{(dso_local)?}} i32* @qux({{.*}} !type [[F_TPTR:![0-9]+]]
+int *qux(char *a, float *b, double *c) {
+  return 0;
+}
+
+// CHECK-DAG: define {{(dso_local)?}} void @corge({{.*}} !type [[F_TVOID]]
+void corge() {
+  int (*fp_baz)(char, float, double) = baz;
+  // ITANIUM: call i32 {{.*}} [ "type"(metadata !"_ZTSFicfdE.generalized") ]
+  // MS:  call i32 {{.*}} [ "type"(metadata !"?6AHDMN@Z.generalized") ]
+  fp_baz('a', .0f, .0);
+
+  int *(*fp

[PATCH] D106172: [clang][CallGraphSection] Add type id metadata to indirect call and targets

[Necip Fazil Yildiran via Phabricator via cfe-commits]

necipfazil created this revision.
Herald added subscribers: dexonsmith, hiraditya.
necipfazil requested review of this revision.
Herald added projects: clang, LLVM.
Herald added subscribers: llvm-commits, cfe-commits.

Create and add generalized type identifier metadata to indirect calls,
and to functions that may be target to indirect calls.

To avoid metadata loss, indirect callsite type identifier metadata are
carried with operand bundles.

Type identifiers will be used by the back-end to construct the call
graph section to precisely represent the possible targets for indirect calls.
The type information is deliberately pulled from the front-end to have extra
precision since some type information is lost at IR, and to ensure
consistent type identifiers between object files compiled at different
times (as C/C++ standards require language-level types to match).


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D106172

Files:
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/test/CodeGen/call-graph-section-1.cpp
  clang/test/CodeGen/call-graph-section-2.cpp
  clang/test/CodeGen/call-graph-section-3.cpp
  clang/test/CodeGen/call-graph-section.c
  llvm/include/llvm/IR/LLVMContext.h
  llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp

Index: llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
===
--- llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
+++ llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
@@ -2877,7 +2877,7 @@
  {LLVMContext::OB_deopt, LLVMContext::OB_gc_transition,
   LLVMContext::OB_gc_live, LLVMContext::OB_funclet,
   LLVMContext::OB_cfguardtarget,
-  LLVMContext::OB_clang_arc_attachedcall}) &&
+  LLVMContext::OB_clang_arc_attachedcall, LLVMContext::OB_type}) &&
  "Cannot lower invokes with arbitrary operand bundles yet!");
 
   const Value *Callee(I.getCalledOperand());
@@ -2960,8 +2960,9 @@
 
   // Deopt bundles are lowered in LowerCallSiteWithDeoptBundle, and we don't
   // have to do anything here to lower funclet bundles.
-  assert(!I.hasOperandBundlesOtherThan(
- {LLVMContext::OB_deopt, LLVMContext::OB_funclet}) &&
+  assert(!I.hasOperandBundlesOtherThan({LLVMContext::OB_deopt,
+LLVMContext::OB_funclet,
+LLVMContext::OB_type}) &&
  "Cannot lower callbrs with arbitrary operand bundles yet!");
 
   assert(I.isInlineAsm() && "Only know how to handle inlineasm callbr");
@@ -8086,7 +8087,7 @@
   assert(!I.hasOperandBundlesOtherThan(
  {LLVMContext::OB_deopt, LLVMContext::OB_funclet,
   LLVMContext::OB_cfguardtarget, LLVMContext::OB_preallocated,
-  LLVMContext::OB_clang_arc_attachedcall}) &&
+  LLVMContext::OB_clang_arc_attachedcall, LLVMContext::OB_type}) &&
  "Cannot lower calls with arbitrary operand bundles!");
 
   SDValue Callee = getValue(I.getCalledOperand());
Index: llvm/include/llvm/IR/LLVMContext.h
===
--- llvm/include/llvm/IR/LLVMContext.h
+++ llvm/include/llvm/IR/LLVMContext.h
@@ -94,6 +94,7 @@
 OB_preallocated = 4,   // "preallocated"
 OB_gc_live = 5,// "gc-live"
 OB_clang_arc_attachedcall = 6, // "clang.arc.attachedcall"
+OB_type = 7,   // "type"
   };
 
   /// getMDKindID - Return a unique non-zero ID for the specified metadata kind.
Index: clang/test/CodeGen/call-graph-section.c
===
--- /dev/null
+++ clang/test/CodeGen/call-graph-section.c
@@ -0,0 +1,85 @@
+// Tests that we assign appropriate identifiers to indirect calls and targets.
+
+// RUN: %clang_cc1 -triple x86_64-unknown-linux -fcall-graph-section -S \
+// RUN: -emit-llvm -o - %s | FileCheck --check-prefixes=CHECK,ITANIUM %s
+
+// RUN: %clang_cc1 -triple x86_64-pc-windows-msvc -fcall-graph-section -S \
+// RUN: -emit-llvm -o - %s | FileCheck --check-prefixes=CHECK,MS %s
+
+// CHECK-DAG: define {{(dso_local)?}} void @foo({{.*}} !type [[F_TVOID:![0-9]+]]
+void foo() {
+}
+
+// CHECK-DAG: define {{(dso_local)?}} void @bar({{.*}} !type [[F_TVOID]]
+void bar() {
+  void (*fp)() = foo;
+  // ITANIUM: call {{.*}} [ "type"(metadata !"_ZTSFvE.generalized") ]
+  // MS:  call {{.*}} [ "type"(metadata !"?6AX@Z.generalized") ]
+  fp();
+}
+
+// CHECK-DAG: define {{(dso_local)?}} i32 @baz({{.*}} !type [[F_TPRIMITIVE:![0-9]+]]
+int baz(char a, float b, double c) {
+  return 1;
+}
+
+// CHECK-DAG: define {{(dso_local)?}} i32* @qux({{.*}} !type [[F_TPTR:![0-9]+]]
+int *qux(char *a, float *b, double *c) {
+  return 0;
+}
+
+// CHECK-DAG: define {{(dso_local)?}} void @corge({{.*}} !type [[F_TVOID]]
+void corge() {
+  int (*fp_baz)(char, float, double) = baz;
+  // ITANIUM: call i32 {{.*}} [ "type"(metadata !"_ZT

[PATCH] D106172: [clang][CallGraphSection] Add type id metadata to indirect call and targets

[Necip Fazil Yildiran via Phabricator via cfe-commits]

necipfazil abandoned this revision.
necipfazil added a comment.

Please disregard this revision.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106172

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D105909: [clang][CallGraphSection] Add type id metadata to indirect call and targets

[Necip Fazil Yildiran via Phabricator via cfe-commits]

necipfazil added a comment.

In D105909#2878378 , @morehouse wrote:

> Also, have you looked into operand bundles 
>  instead of metadata?  I don't 
> know much about them, but they might help with the 
> optimizations-dropping-metadata problem.

Latest changes switches to using operand bundles instead of metadata. Besides 
solving the optimizations-dropping-metadata problem, it simplified the code as 
well.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105909

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Necip Fazil Yildiran via Phabricator via cfe-commits]

necipfazil updated this revision to Diff 359592.
necipfazil added a comment.

Change call graph section layout

- Changed the call graph section layout
- Extended the example
- Updated the documentation

In the previous version, the call graph section layout was optimized for
entries with unique type ids in which to list any indirect calls and targets
with such type id. However, the implementation requirements prevented sharing
information from different functions in the same entry. The main reason is
the need for separate entries per functions to allow dead-stripping.

As the previous layout was suboptimal for per-function call graph section
entries, a new layout is created. Please see the updated documentation for the
new layout.

The new layout has the following advantages:

- The previous layout allowed multiple indirect targets to be listed in a 
single entry. However, there was no use since each entry was created for a 
single function. With the new layout, an entry is said to belong to a function, 
and lists info on the function and its indirect callsites. Consequently, the 
layout is designed for per-function entry approach.
- The previous layout listed callsites per type id as `(TypeId, 
CallSitesWithTypeIdCount, [CallSite1WithTypeId, ...])`. However, now that the 
callsites are listed per function, a small number of callsites are expected for 
each type id. Therefore, they are now listed as `(TypeId, CallSite)` pairs to 
avoid wasting space on count values that are usually 1.
- In the previous layout, if a function was not an indirect target, its entry 
PC was not included. This prevented making the distinction between
  1. the non-indirect target functions and 2) those functions that we had no 
information about (e.g., functions from linked objects with no call graph 
section). With the new layout, the function entry PC is always listed. It is 
marked as non-indirect target if so. For completeness, the user still needs to 
take any function with missing information as receiver to any indirect calls. 
However, the user can avoid doing the same for the non-indirect targets as 
their entry PCs are now listed, which improves the precision for the resulting 
call graph.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105907

Files:
  clang/docs/CallGraphSection.rst
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/clang_f_opts.c
  llvm/include/llvm/CodeGen/CommandFlags.h
  llvm/include/llvm/Target/TargetOptions.h
  llvm/lib/CodeGen/CommandFlags.cpp

Index: llvm/lib/CodeGen/CommandFlags.cpp
===
--- llvm/lib/CodeGen/CommandFlags.cpp
+++ llvm/lib/CodeGen/CommandFlags.cpp
@@ -86,6 +86,7 @@
 CGOPT(DebuggerKind, DebuggerTuningOpt)
 CGOPT(bool, EnableStackSizeSection)
 CGOPT(bool, EnableAddrsig)
+CGOPT(bool, EnableCallGraphSection)
 CGOPT(bool, EmitCallSiteInfo)
 CGOPT(bool, EnableMachineFunctionSplitter)
 CGOPT(bool, EnableDebugEntryValues)
@@ -407,6 +408,11 @@
   cl::init(false));
   CGBINDOPT(EnableAddrsig);
 
+  static cl::opt EnableCallGraphSection(
+  "call-graph-section", cl::desc("Emit a call graph section"),
+  cl::init(false));
+  CGBINDOPT(EnableCallGraphSection);
+
   static cl::opt EmitCallSiteInfo(
   "emit-call-site-info",
   cl::desc(
@@ -520,6 +526,7 @@
   Options.EmitStackSizeSection = getEnableStackSizeSection();
   Options.EnableMachineFunctionSplitter = getEnableMachineFunctionSplitter();
   Options.EmitAddrsig = getEnableAddrsig();
+  Options.EmitCallGraphSection = getEnableCallGraphSection();
   Options.EmitCallSiteInfo = getEmitCallSiteInfo();
   Options.EnableDebugEntryValues = getEnableDebugEntryValues();
   Options.PseudoProbeForProfiling = getPseudoProbeForProfiling();
Index: llvm/include/llvm/Target/TargetOptions.h
===
--- llvm/include/llvm/Target/TargetOptions.h
+++ llvm/include/llvm/Target/TargetOptions.h
@@ -127,11 +127,11 @@
   EmulatedTLS(false), ExplicitEmulatedTLS(false), EnableIPRA(false),
   EmitStackSizeSection(false), EnableMachineOutliner(false),
   EnableMachineFunctionSplitter(false), SupportsDefaultOutlining(false),
-  EmitAddrsig(false), EmitCallSiteInfo(false),
-  SupportsDebugEntryValues(false), EnableDebugEntryValues(false),
-  PseudoProbeForProfiling(false), ValueTrackingVariableLocations(false),
-  ForceDwarfFrameSection(false), XRayOmitFunctionIndex(false),
-  DebugStrictDwarf(false),
+  EmitAddrsig(false), EmitCallGraphSection(false),
+  EmitCallSiteInfo(false), SupportsDebugEntryValues(false),
+  EnableDebugEntryValues(false), PseudoProbeForProfiling(false),
+  ValueTrackingVariableLocations(false), F

[PATCH] D105911: [CallGraphSection] Introduce CGSectionFuncComdatCreator pass

[Necip Fazil Yildiran via Phabricator via cfe-commits]

necipfazil abandoned this revision.
necipfazil added a comment.

In D105911#2878345 , @morehouse wrote:

> Are comdats needed?  Can we get proper dead stripping with just 
> `SHF_LINK_ORDER`?

It looks like we can. I am abandoning this revision. I will shortly push the 
changes to related revisions for not using comdats.

> @MaskRay recently updated the documentation for associated metadata 
>  to imply that our 
> symbol doesn't need to share a comdat with its associated function when the 
> function doesn't have a comdat.
>
> Also, @MaskRay: Can adding comdats like this change the final code in the 
> fully-linked binary?




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105911

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D105909: [clang][CallGraphSection] Add type id metadata to indirect call and targets

[Necip Fazil Yildiran via Phabricator via cfe-commits]

necipfazil updated this revision to Diff 360875.
necipfazil marked 2 inline comments as done.
necipfazil added a comment.

Fix lint


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105909

Files:
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/test/CodeGen/call-graph-section-1.cpp
  clang/test/CodeGen/call-graph-section-2.cpp
  clang/test/CodeGen/call-graph-section-3.cpp
  clang/test/CodeGen/call-graph-section.c
  llvm/include/llvm/IR/LLVMContext.h
  llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp

Index: llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
===
--- llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
+++ llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
@@ -2877,7 +2877,7 @@
  {LLVMContext::OB_deopt, LLVMContext::OB_gc_transition,
   LLVMContext::OB_gc_live, LLVMContext::OB_funclet,
   LLVMContext::OB_cfguardtarget,
-  LLVMContext::OB_clang_arc_attachedcall}) &&
+  LLVMContext::OB_clang_arc_attachedcall, LLVMContext::OB_type}) &&
  "Cannot lower invokes with arbitrary operand bundles yet!");
 
   const Value *Callee(I.getCalledOperand());
@@ -2960,8 +2960,9 @@
 
   // Deopt bundles are lowered in LowerCallSiteWithDeoptBundle, and we don't
   // have to do anything here to lower funclet bundles.
-  assert(!I.hasOperandBundlesOtherThan(
- {LLVMContext::OB_deopt, LLVMContext::OB_funclet}) &&
+  assert(!I.hasOperandBundlesOtherThan({LLVMContext::OB_deopt,
+LLVMContext::OB_funclet,
+LLVMContext::OB_type}) &&
  "Cannot lower callbrs with arbitrary operand bundles yet!");
 
   assert(I.isInlineAsm() && "Only know how to handle inlineasm callbr");
@@ -8086,7 +8087,7 @@
   assert(!I.hasOperandBundlesOtherThan(
  {LLVMContext::OB_deopt, LLVMContext::OB_funclet,
   LLVMContext::OB_cfguardtarget, LLVMContext::OB_preallocated,
-  LLVMContext::OB_clang_arc_attachedcall}) &&
+  LLVMContext::OB_clang_arc_attachedcall, LLVMContext::OB_type}) &&
  "Cannot lower calls with arbitrary operand bundles!");
 
   SDValue Callee = getValue(I.getCalledOperand());
Index: llvm/include/llvm/IR/LLVMContext.h
===
--- llvm/include/llvm/IR/LLVMContext.h
+++ llvm/include/llvm/IR/LLVMContext.h
@@ -94,6 +94,7 @@
 OB_preallocated = 4,   // "preallocated"
 OB_gc_live = 5,// "gc-live"
 OB_clang_arc_attachedcall = 6, // "clang.arc.attachedcall"
+OB_type = 7,   // "type"
   };
 
   /// getMDKindID - Return a unique non-zero ID for the specified metadata kind.
Index: clang/test/CodeGen/call-graph-section.c
===
--- /dev/null
+++ clang/test/CodeGen/call-graph-section.c
@@ -0,0 +1,85 @@
+// Tests that we assign appropriate identifiers to indirect calls and targets.
+
+// RUN: %clang_cc1 -triple x86_64-unknown-linux -fcall-graph-section -S \
+// RUN: -emit-llvm -o - %s | FileCheck --check-prefixes=CHECK,ITANIUM %s
+
+// RUN: %clang_cc1 -triple x86_64-pc-windows-msvc -fcall-graph-section -S \
+// RUN: -emit-llvm -o - %s | FileCheck --check-prefixes=CHECK,MS %s
+
+// CHECK-DAG: define {{(dso_local)?}} void @foo({{.*}} !type [[F_TVOID:![0-9]+]]
+void foo() {
+}
+
+// CHECK-DAG: define {{(dso_local)?}} void @bar({{.*}} !type [[F_TVOID]]
+void bar() {
+  void (*fp)() = foo;
+  // ITANIUM: call {{.*}} [ "type"(metadata !"_ZTSFvE.generalized") ]
+  // MS:  call {{.*}} [ "type"(metadata !"?6AX@Z.generalized") ]
+  fp();
+}
+
+// CHECK-DAG: define {{(dso_local)?}} i32 @baz({{.*}} !type [[F_TPRIMITIVE:![0-9]+]]
+int baz(char a, float b, double c) {
+  return 1;
+}
+
+// CHECK-DAG: define {{(dso_local)?}} i32* @qux({{.*}} !type [[F_TPTR:![0-9]+]]
+int *qux(char *a, float *b, double *c) {
+  return 0;
+}
+
+// CHECK-DAG: define {{(dso_local)?}} void @corge({{.*}} !type [[F_TVOID]]
+void corge() {
+  int (*fp_baz)(char, float, double) = baz;
+  // ITANIUM: call i32 {{.*}} [ "type"(metadata !"_ZTSFicfdE.generalized") ]
+  // MS:  call i32 {{.*}} [ "type"(metadata !"?6AHDMN@Z.generalized") ]
+  fp_baz('a', .0f, .0);
+
+  int *(*fp_qux)(char *, float *, double *) = qux;
+  // ITANIUM: call i32* {{.*}} [ "type"(metadata !"_ZTSFPvS_S_S_E.generalized") ]
+  // MS:  call i32* {{.*}} [ "type"(metadata !"?6APEAXPEAX00@Z.generalized") ]
+  fp_qux(0, 0, 0);
+}
+
+struct st1 {
+  int *(*fp)(char *, float *, double *);
+};
+
+struct st2 {
+  struct st1 m;
+};
+
+// CHECK-DAG: define {{(dso_local)?}} void @stparam({{.*}} !type [[F_TSTRUCT:![0-9]+]]
+void stparam(struct st2 a, struct st2 *b) {}
+
+// CHECK-DAG: define {{(dso_local)?}} void @stf({{.*}} !type [[F_TVO

[PATCH] D105909: [clang][CallGraphSection] Add type id metadata to indirect call and targets

[Necip Fazil Yildiran via Phabricator via cfe-commits]

necipfazil added a comment.

@morehouse PTAL




Comment at: llvm/include/llvm/IR/LLVMContext.h:97
 OB_clang_arc_attachedcall = 6, // "clang.arc.attachedcall"
+OB_type = 7,   // "type"
   };

morehouse wrote:
> Do we need to update `LLVMContext::LLVMContext()`?
Operand bundles are originally used as input to lowering in 
`SelectionDAGBuilder`. Arbitrary operand bundles are not allowed.

To allow `type` operand bundle, we need this change. Please see 
`llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp` for related changes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105909

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D105909: [clang][CallGraphSection] Add type id metadata to indirect call and targets

[Necip Fazil Yildiran via Phabricator via cfe-commits]

necipfazil updated this revision to Diff 360942.
necipfazil added a comment.

Verify type operand bundle usage

- Check if type operand bundle id is drifted
- Verify and complain if multiple type operand bundles are used
- Add type operand bundle test to operand bundle verifier tests


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105909

Files:
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/test/CodeGen/call-graph-section-1.cpp
  clang/test/CodeGen/call-graph-section-2.cpp
  clang/test/CodeGen/call-graph-section-3.cpp
  clang/test/CodeGen/call-graph-section.c
  llvm/include/llvm/IR/LLVMContext.h
  llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
  llvm/lib/IR/LLVMContext.cpp
  llvm/lib/IR/Verifier.cpp
  llvm/test/Verifier/operand-bundles.ll

Index: llvm/test/Verifier/operand-bundles.ll
===
--- llvm/test/Verifier/operand-bundles.ll
+++ llvm/test/Verifier/operand-bundles.ll
@@ -81,4 +81,17 @@
   ret void
 }
 
+define void @f_type(i32* %ptr) {
+; CHECK: Multiple "type" operand bundles
+; CHECK-NEXT: call void @g() [ "type"(metadata !"_ZTSFvE.generalized"), "type"(metadata !"_ZTSFvE.generalized") ]
+; CHECK-NOT: call void @g() [ "type"(metadata !"_ZTSFvE.generalized") ]
+
+ entry:
+  %l = load i32, i32* %ptr
+   call void @g() [ "type"(metadata !"_ZTSFvE.generalized"), "type"(metadata !"_ZTSFvE.generalized") ]
+  call void @g() [ "type"(metadata !"_ZTSFvE.generalized") ]
+  %x = add i32 42, 1
+  ret void
+}
+
 attributes #0 = { noreturn }
Index: llvm/lib/IR/Verifier.cpp
===
--- llvm/lib/IR/Verifier.cpp
+++ llvm/lib/IR/Verifier.cpp
@@ -3205,12 +3205,12 @@
   visitIntrinsicCall(ID, Call);
 
   // Verify that a callsite has at most one "deopt", at most one "funclet", at
-  // most one "gc-transition", at most one "cfguardtarget",
+  // most one "gc-transition", at most one "cfguardtarget", at most one "type",
   // and at most one "preallocated" operand bundle.
   bool FoundDeoptBundle = false, FoundFuncletBundle = false,
FoundGCTransitionBundle = false, FoundCFGuardTargetBundle = false,
FoundPreallocatedBundle = false, FoundGCLiveBundle = false,
-   FoundAttachedCallBundle = false;
+   FoundAttachedCallBundle = false, FoundTypeBundle = false;
   for (unsigned i = 0, e = Call.getNumOperandBundles(); i < e; ++i) {
 OperandBundleUse BU = Call.getOperandBundleAt(i);
 uint32_t Tag = BU.getTagID();
@@ -3255,6 +3255,9 @@
   Assert(!FoundAttachedCallBundle,
  "Multiple \"clang.arc.attachedcall\" operand bundles", Call);
   FoundAttachedCallBundle = true;
+} else if (Tag == LLVMContext::OB_type) {
+  Assert(!FoundTypeBundle, "Multiple \"type\" operand bundles", Call);
+  FoundTypeBundle = true;
 }
   }
 
Index: llvm/lib/IR/LLVMContext.cpp
===
--- llvm/lib/IR/LLVMContext.cpp
+++ llvm/lib/IR/LLVMContext.cpp
@@ -84,6 +84,11 @@
  "clang.arc.attachedcall operand bundle id drifted!");
   (void)ClangAttachedCall;
 
+  auto *TypeEntry = pImpl->getOrInsertBundleTag("type");
+  assert(TypeEntry->second == LLVMContext::OB_type &&
+ "type operand bundle id drifted!");
+  (void)TypeEntry;
+
   SyncScope::ID SingleThreadSSID =
   pImpl->getOrInsertSyncScopeID("singlethread");
   assert(SingleThreadSSID == SyncScope::SingleThread &&
Index: llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
===
--- llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
+++ llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
@@ -2877,7 +2877,7 @@
  {LLVMContext::OB_deopt, LLVMContext::OB_gc_transition,
   LLVMContext::OB_gc_live, LLVMContext::OB_funclet,
   LLVMContext::OB_cfguardtarget,
-  LLVMContext::OB_clang_arc_attachedcall}) &&
+  LLVMContext::OB_clang_arc_attachedcall, LLVMContext::OB_type}) &&
  "Cannot lower invokes with arbitrary operand bundles yet!");
 
   const Value *Callee(I.getCalledOperand());
@@ -2960,8 +2960,9 @@
 
   // Deopt bundles are lowered in LowerCallSiteWithDeoptBundle, and we don't
   // have to do anything here to lower funclet bundles.
-  assert(!I.hasOperandBundlesOtherThan(
- {LLVMContext::OB_deopt, LLVMContext::OB_funclet}) &&
+  assert(!I.hasOperandBundlesOtherThan({LLVMContext::OB_deopt,
+LLVMContext::OB_funclet,
+LLVMContext::OB_type}) &&
  "Cannot lower callbrs with arbitrary operand bundles yet!");
 
   assert(I.isInlineAsm() && "Only know how to handle inlineasm callbr");
@@ -8086,7 +8087,7 @@
   assert(!I.hasOperandBundlesOtherThan(
  {LLVMContext::OB_deopt, LLVMContext::O

[PATCH] D105909: [clang][CallGraphSection] Add type id metadata to indirect call and targets

[Necip Fazil Yildiran via Phabricator via cfe-commits]

necipfazil marked an inline comment as done.
necipfazil added inline comments.



Comment at: llvm/include/llvm/IR/LLVMContext.h:97
 OB_clang_arc_attachedcall = 6, // "clang.arc.attachedcall"
+OB_type = 7,   // "type"
   };

morehouse wrote:
> necipfazil wrote:
> > morehouse wrote:
> > > Do we need to update `LLVMContext::LLVMContext()`?
> > Operand bundles are originally used as input to lowering in 
> > `SelectionDAGBuilder`. Arbitrary operand bundles are not allowed.
> > 
> > To allow `type` operand bundle, we need this change. Please see 
> > `llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp` for related changes.
> So why don't you need to update `LLVMContext::LLVMContext()` then?  All other 
> operand bundle types are there.
I added further verifications in LLVMContext::LLVMContext() and some tests for 
it.

If we requested updates are for somewhere else, could you please provide more 
details?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105909

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Necip Fazil Yildiran via Phabricator via cfe-commits]

necipfazil updated this revision to Diff 361091.
necipfazil marked 10 inline comments as done.
necipfazil added a comment.

Fix nits in documentation and tests


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105907

Files:
  clang/docs/CallGraphSection.rst
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/call-graph-section.c
  llvm/include/llvm/CodeGen/CommandFlags.h
  llvm/include/llvm/Target/TargetOptions.h
  llvm/lib/CodeGen/CommandFlags.cpp

Index: llvm/lib/CodeGen/CommandFlags.cpp
===
--- llvm/lib/CodeGen/CommandFlags.cpp
+++ llvm/lib/CodeGen/CommandFlags.cpp
@@ -86,6 +86,7 @@
 CGOPT(DebuggerKind, DebuggerTuningOpt)
 CGOPT(bool, EnableStackSizeSection)
 CGOPT(bool, EnableAddrsig)
+CGOPT(bool, EnableCallGraphSection)
 CGOPT(bool, EmitCallSiteInfo)
 CGOPT(bool, EnableMachineFunctionSplitter)
 CGOPT(bool, EnableDebugEntryValues)
@@ -407,6 +408,11 @@
   cl::init(false));
   CGBINDOPT(EnableAddrsig);
 
+  static cl::opt EnableCallGraphSection(
+  "call-graph-section", cl::desc("Emit a call graph section"),
+  cl::init(false));
+  CGBINDOPT(EnableCallGraphSection);
+
   static cl::opt EmitCallSiteInfo(
   "emit-call-site-info",
   cl::desc(
@@ -520,6 +526,7 @@
   Options.EmitStackSizeSection = getEnableStackSizeSection();
   Options.EnableMachineFunctionSplitter = getEnableMachineFunctionSplitter();
   Options.EmitAddrsig = getEnableAddrsig();
+  Options.EmitCallGraphSection = getEnableCallGraphSection();
   Options.EmitCallSiteInfo = getEmitCallSiteInfo();
   Options.EnableDebugEntryValues = getEnableDebugEntryValues();
   Options.PseudoProbeForProfiling = getPseudoProbeForProfiling();
Index: llvm/include/llvm/Target/TargetOptions.h
===
--- llvm/include/llvm/Target/TargetOptions.h
+++ llvm/include/llvm/Target/TargetOptions.h
@@ -127,11 +127,11 @@
   EmulatedTLS(false), ExplicitEmulatedTLS(false), EnableIPRA(false),
   EmitStackSizeSection(false), EnableMachineOutliner(false),
   EnableMachineFunctionSplitter(false), SupportsDefaultOutlining(false),
-  EmitAddrsig(false), EmitCallSiteInfo(false),
-  SupportsDebugEntryValues(false), EnableDebugEntryValues(false),
-  PseudoProbeForProfiling(false), ValueTrackingVariableLocations(false),
-  ForceDwarfFrameSection(false), XRayOmitFunctionIndex(false),
-  DebugStrictDwarf(false),
+  EmitAddrsig(false), EmitCallGraphSection(false),
+  EmitCallSiteInfo(false), SupportsDebugEntryValues(false),
+  EnableDebugEntryValues(false), PseudoProbeForProfiling(false),
+  ValueTrackingVariableLocations(false), ForceDwarfFrameSection(false),
+  XRayOmitFunctionIndex(false), DebugStrictDwarf(false),
   FPDenormalMode(DenormalMode::IEEE, DenormalMode::IEEE) {}
 
 /// DisableFramePointerElim - This returns true if frame pointer elimination
@@ -290,6 +290,9 @@
 /// to selectively generate basic block sections.
 std::shared_ptr BBSectionsFuncListBuf;
 
+/// Emit section containing call graph metadata.
+unsigned EmitCallGraphSection : 1;
+
 /// The flag enables call site info production. It is used only for debug
 /// info, and it is restricted only to optimized code. This can be used for
 /// something else, so that should be controlled in the frontend.
Index: llvm/include/llvm/CodeGen/CommandFlags.h
===
--- llvm/include/llvm/CodeGen/CommandFlags.h
+++ llvm/include/llvm/CodeGen/CommandFlags.h
@@ -122,6 +122,8 @@
 
 bool getEnableAddrsig();
 
+bool getEnableCallGraphSection();
+
 bool getEmitCallSiteInfo();
 
 bool getEnableMachineFunctionSplitter();
Index: clang/test/Driver/call-graph-section.c
===
--- /dev/null
+++ clang/test/Driver/call-graph-section.c
@@ -0,0 +1,5 @@
+// RUN: %clang -### -S -fcall-graph-section %s 2>&1 | FileCheck --check-prefix=CALL-GRAPH-SECTION %s
+// RUN: %clang -### -S -fcall-graph-section -fno-call-graph-section %s 2>&1 | FileCheck --check-prefix=NO-CALL-GRAPH-SECTION %s
+
+// CALL-GRAPH-SECTION: "-fcall-graph-section"
+// NO-CALL-GRAPH-SECTION-NOT: "-fcall-graph-section"
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -5576,6 +5576,10 @@
 CmdArgs.push_back(A->getValue());
   }
 
+  if (Args.hasFlag(options::OPT_fcall_graph_section,
+   options::OPT_fno_call_graph_section, false))
+CmdArgs.push_back("-fcall-graph-section");
+
   if (Args.hasFlag(options::O

[PATCH] D105909: [clang][CallGraphSection] Add type id metadata to indirect call and targets

[Necip Fazil Yildiran via Phabricator via cfe-commits]

necipfazil updated this revision to Diff 362652.
necipfazil marked 2 inline comments as done.
necipfazil added a comment.

Fix nit


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105909

Files:
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/test/CodeGen/call-graph-section-1.cpp
  clang/test/CodeGen/call-graph-section-2.cpp
  clang/test/CodeGen/call-graph-section-3.cpp
  clang/test/CodeGen/call-graph-section.c
  llvm/include/llvm/IR/LLVMContext.h
  llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
  llvm/lib/IR/LLVMContext.cpp
  llvm/lib/IR/Verifier.cpp
  llvm/test/Verifier/operand-bundles.ll

Index: llvm/test/Verifier/operand-bundles.ll
===
--- llvm/test/Verifier/operand-bundles.ll
+++ llvm/test/Verifier/operand-bundles.ll
@@ -81,4 +81,17 @@
   ret void
 }
 
+define void @f_type(i32* %ptr) {
+; CHECK: Multiple "type" operand bundles
+; CHECK-NEXT: call void @g() [ "type"(metadata !"_ZTSFvE.generalized"), "type"(metadata !"_ZTSFvE.generalized") ]
+; CHECK-NOT: call void @g() [ "type"(metadata !"_ZTSFvE.generalized") ]
+
+ entry:
+  %l = load i32, i32* %ptr
+  call void @g() [ "type"(metadata !"_ZTSFvE.generalized"), "type"(metadata !"_ZTSFvE.generalized") ]
+  call void @g() [ "type"(metadata !"_ZTSFvE.generalized") ]
+  %x = add i32 42, 1
+  ret void
+}
+
 attributes #0 = { noreturn }
Index: llvm/lib/IR/Verifier.cpp
===
--- llvm/lib/IR/Verifier.cpp
+++ llvm/lib/IR/Verifier.cpp
@@ -3205,12 +3205,12 @@
   visitIntrinsicCall(ID, Call);
 
   // Verify that a callsite has at most one "deopt", at most one "funclet", at
-  // most one "gc-transition", at most one "cfguardtarget",
+  // most one "gc-transition", at most one "cfguardtarget", at most one "type",
   // and at most one "preallocated" operand bundle.
   bool FoundDeoptBundle = false, FoundFuncletBundle = false,
FoundGCTransitionBundle = false, FoundCFGuardTargetBundle = false,
FoundPreallocatedBundle = false, FoundGCLiveBundle = false,
-   FoundAttachedCallBundle = false;
+   FoundAttachedCallBundle = false, FoundTypeBundle = false;
   for (unsigned i = 0, e = Call.getNumOperandBundles(); i < e; ++i) {
 OperandBundleUse BU = Call.getOperandBundleAt(i);
 uint32_t Tag = BU.getTagID();
@@ -3255,6 +3255,9 @@
   Assert(!FoundAttachedCallBundle,
  "Multiple \"clang.arc.attachedcall\" operand bundles", Call);
   FoundAttachedCallBundle = true;
+} else if (Tag == LLVMContext::OB_type) {
+  Assert(!FoundTypeBundle, "Multiple \"type\" operand bundles", Call);
+  FoundTypeBundle = true;
 }
   }
 
Index: llvm/lib/IR/LLVMContext.cpp
===
--- llvm/lib/IR/LLVMContext.cpp
+++ llvm/lib/IR/LLVMContext.cpp
@@ -84,6 +84,11 @@
  "clang.arc.attachedcall operand bundle id drifted!");
   (void)ClangAttachedCall;
 
+  auto *TypeEntry = pImpl->getOrInsertBundleTag("type");
+  assert(TypeEntry->second == LLVMContext::OB_type &&
+ "type operand bundle id drifted!");
+  (void)TypeEntry;
+
   SyncScope::ID SingleThreadSSID =
   pImpl->getOrInsertSyncScopeID("singlethread");
   assert(SingleThreadSSID == SyncScope::SingleThread &&
Index: llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
===
--- llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
+++ llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
@@ -2877,7 +2877,7 @@
  {LLVMContext::OB_deopt, LLVMContext::OB_gc_transition,
   LLVMContext::OB_gc_live, LLVMContext::OB_funclet,
   LLVMContext::OB_cfguardtarget,
-  LLVMContext::OB_clang_arc_attachedcall}) &&
+  LLVMContext::OB_clang_arc_attachedcall, LLVMContext::OB_type}) &&
  "Cannot lower invokes with arbitrary operand bundles yet!");
 
   const Value *Callee(I.getCalledOperand());
@@ -2960,8 +2960,9 @@
 
   // Deopt bundles are lowered in LowerCallSiteWithDeoptBundle, and we don't
   // have to do anything here to lower funclet bundles.
-  assert(!I.hasOperandBundlesOtherThan(
- {LLVMContext::OB_deopt, LLVMContext::OB_funclet}) &&
+  assert(!I.hasOperandBundlesOtherThan({LLVMContext::OB_deopt,
+LLVMContext::OB_funclet,
+LLVMContext::OB_type}) &&
  "Cannot lower callbrs with arbitrary operand bundles yet!");
 
   assert(I.isInlineAsm() && "Only know how to handle inlineasm callbr");
@@ -8086,7 +8087,7 @@
   assert(!I.hasOperandBundlesOtherThan(
  {LLVMContext::OB_deopt, LLVMContext::OB_funclet,
   LLVMContext::OB_cfguardtarget, LLVMContext::OB_preallocated,
-  LLVMContext::OB_clang_arc_attachedcall}) &&
+

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

[Necip Fazil Yildiran via Phabricator via cfe-commits]

necipfazil created this revision.
necipfazil added reviewers: rsmith, morehouse, kcc, cfe-commits, llvm-commits.
Herald added subscribers: ormris, dexonsmith, dang, hiraditya.
necipfazil requested review of this revision.
Herald added projects: clang, LLVM.

This is the first of the patch series that adds the support for
computing, storing, and restoring call graphs with LLVM. This adds the
options and the design documentation for computing and storing the call
graphs.

  

Inferring indirect call targets from a binary is challenging without
source-level information. Hence, the reconstruction of a fine-grained
call graph from the binary is unfeasible for indirect/virtual calls.
To address this, designed solution is to collect the necessary information
to construct the call graph while the source information is present, and
store it in a non-code section of the binary. To enable, use
-fcall-graph-section for Clang, or --call-graph-section for LLVM.

Original RFC: https://lists.llvm.org/pipermail/llvm-dev/2021-June/151044.html
Updated RFC: https://lists.llvm.org/pipermail/llvm-dev/2021-July/151739.html


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D105907

Files:
  clang/docs/CallGraphSection.rst
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  llvm/include/llvm/CodeGen/CommandFlags.h
  llvm/include/llvm/Target/TargetOptions.h
  llvm/lib/CodeGen/CommandFlags.cpp

Index: llvm/lib/CodeGen/CommandFlags.cpp
===
--- llvm/lib/CodeGen/CommandFlags.cpp
+++ llvm/lib/CodeGen/CommandFlags.cpp
@@ -86,6 +86,7 @@
 CGOPT(DebuggerKind, DebuggerTuningOpt)
 CGOPT(bool, EnableStackSizeSection)
 CGOPT(bool, EnableAddrsig)
+CGOPT(bool, EnableCallGraphSection)
 CGOPT(bool, EmitCallSiteInfo)
 CGOPT(bool, EnableMachineFunctionSplitter)
 CGOPT(bool, EnableDebugEntryValues)
@@ -407,6 +408,11 @@
   cl::init(false));
   CGBINDOPT(EnableAddrsig);
 
+  static cl::opt EnableCallGraphSection(
+  "call-graph-section", cl::desc("Emit a call graph section"),
+  cl::init(false));
+  CGBINDOPT(EnableCallGraphSection);
+
   static cl::opt EmitCallSiteInfo(
   "emit-call-site-info",
   cl::desc(
@@ -520,6 +526,7 @@
   Options.EmitStackSizeSection = getEnableStackSizeSection();
   Options.EnableMachineFunctionSplitter = getEnableMachineFunctionSplitter();
   Options.EmitAddrsig = getEnableAddrsig();
+  Options.EmitCallGraphSection = getEnableCallGraphSection();
   Options.EmitCallSiteInfo = getEmitCallSiteInfo();
   Options.EnableDebugEntryValues = getEnableDebugEntryValues();
   Options.PseudoProbeForProfiling = getPseudoProbeForProfiling();
Index: llvm/include/llvm/Target/TargetOptions.h
===
--- llvm/include/llvm/Target/TargetOptions.h
+++ llvm/include/llvm/Target/TargetOptions.h
@@ -127,11 +127,11 @@
   EmulatedTLS(false), ExplicitEmulatedTLS(false), EnableIPRA(false),
   EmitStackSizeSection(false), EnableMachineOutliner(false),
   EnableMachineFunctionSplitter(false), SupportsDefaultOutlining(false),
-  EmitAddrsig(false), EmitCallSiteInfo(false),
-  SupportsDebugEntryValues(false), EnableDebugEntryValues(false),
-  PseudoProbeForProfiling(false), ValueTrackingVariableLocations(false),
-  ForceDwarfFrameSection(false), XRayOmitFunctionIndex(false),
-  DebugStrictDwarf(false),
+  EmitAddrsig(false), EmitCallGraphSection(false),
+  EmitCallSiteInfo(false), SupportsDebugEntryValues(false),
+  EnableDebugEntryValues(false), PseudoProbeForProfiling(false),
+  ValueTrackingVariableLocations(false), ForceDwarfFrameSection(false),
+  XRayOmitFunctionIndex(false), DebugStrictDwarf(false),
   FPDenormalMode(DenormalMode::IEEE, DenormalMode::IEEE) {}
 
 /// DisableFramePointerElim - This returns true if frame pointer elimination
@@ -290,6 +290,9 @@
 /// to selectively generate basic block sections.
 std::shared_ptr BBSectionsFuncListBuf;
 
+/// Emit section containing call graph metadata.
+unsigned EmitCallGraphSection : 1;
+
 /// The flag enables call site info production. It is used only for debug
 /// info, and it is restricted only to optimized code. This can be used for
 /// something else, so that should be controlled in the frontend.
Index: llvm/include/llvm/CodeGen/CommandFlags.h
===
--- llvm/include/llvm/CodeGen/CommandFlags.h
+++ llvm/include/llvm/CodeGen/CommandFlags.h
@@ -122,6 +122,8 @@
 
 bool getEnableAddrsig();
 
+bool getEnableCallGraphSection();
+
 bool getEmitCallSiteInfo();
 
 bool getEnableMachineFunctionSplitter();
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Drive

[PATCH] D105909: [clang][CallGraphSection] Add type id metadata to indirect call and targets

[Necip Fazil Yildiran via Phabricator via cfe-commits]

necipfazil created this revision.
necipfazil added reviewers: rjmccall, rsmith, morehouse, kcc, llvm-commits, 
cfe-commits.
necipfazil requested review of this revision.
Herald added a project: clang.

Create and add generalized type identifier metadata to indirect calls,
and to functions that may be target to indirect calls.

  

Type identifiers will be used by the back-end to construct the call
graph section to precisely represent the possible targets for indirect calls.
The type information is deliberately pulled from the front-end to have extra
precision since some type information is lost at IR, and to ensure
consistent type identifiers between object files compiled at different
times (as C/C++ standards require language-level types to match).

Original RFC: https://lists.llvm.org/pipermail/llvm-dev/2021-June/151044.html
Updated RFC: https://lists.llvm.org/pipermail/llvm-dev/2021-July/151739.html


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D105909

Files:
  clang/lib/CodeGen/CGClass.cpp
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/CodeGen/CGExprCXX.cpp
  clang/lib/CodeGen/CGObjCMac.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/CodeGen/CodeGenModule.h
  clang/test/CodeGen/call-graph-section.c

Index: clang/test/CodeGen/call-graph-section.c
===
--- /dev/null
+++ clang/test/CodeGen/call-graph-section.c
@@ -0,0 +1,47 @@
+// Tests that we assign appropriate identifiers to indirect calls and targets.
+
+// RUN: %clang_cc1 -triple x86_64-unknown-linux -fcall-graph-section -S \
+// RUN: -emit-llvm -o - %s | FileCheck --check-prefix=CHECK --check-prefix=ITANIUM %s
+
+// RUN: %clang_cc1 -triple x86_64-pc-windows-msvc -fcall-graph-section -S \
+// RUN: -emit-llvm -o - %s | FileCheck --check-prefix=CHECK --check-prefix=MS %s
+
+// CHECK: define {{(dso_local )?}}void @foo({{.*}} !type [[TVOID_GENERALIZED:![0-9]+]]
+void foo() {
+}
+
+// CHECK: define {{(dso_local )?}}void @bar({{.*}} !type [[TVOID_GENERALIZED:![0-9]+]]
+void bar() {
+  void (*fp)() = foo;
+  // CHECK: call {{.*}} !type [[TVOID_GENERALIZED:![0-9]+]]
+  return fp();
+}
+
+// CHECK: define {{(dso_local )?}}i32 @baz({{.*}} !type [[TPRIMITIVE_GENERALIZED:![0-9]+]]
+int baz(char a, float b, double c) {
+  return 1;
+}
+
+// CHECK: define {{(dso_local )?}}i32* @qux({{.*}} !type [[TPTR_GENERALIZED:![0-9]+]]
+int *qux(char *a, float *b, double *c) {
+  return 0;
+}
+
+// CHECK: define {{(dso_local )?}}void @corge({{.*}} !type [[TVOID_GENERALIZED:![0-9]+]]
+void corge() {
+  int (*fp_baz)(char, float, double) = baz;
+  // CHECK: call i32 {{.*}}, !type [[TPRIMITIVE_GENERALIZED:![0-9]+]]
+  fp_baz('a', .0f, .0);
+
+  int *(*fp_qux)(char *, float *, double *) = qux;
+  // CHECK: call i32* {{.*}}, !type [[TPTR_GENERALIZED:![0-9]+]]
+  fp_qux(0, 0, 0);
+}
+
+// ITANIUM-DAG: [[TVOID_GENERALIZED]] = !{{{.*}}!"_ZTSFvE.generalized"}
+// ITANIUM-DAG: [[TPRIMITIVE_GENERALIZED]] = !{{{.*}}!"_ZTSFicfdE.generalized"}
+// ITANIUM-DAG: [[TPTR_GENERALIZED]] = !{{{.*}}!"_ZTSFPvS_S_S_E.generalized"}
+// MS-DAG: [[TVOID_GENERALIZED]] = !{{{.*}}!"?6AX@Z.generalized"}
+// MS-DAG: [[TPRIMITIVE_GENERALIZED]] = !{{{.*}}!"?6AHDMN@Z.generalized"}
+// MS-DAG: [[TPTR_GENERALIZED]] = !{{{.*}}!"?6APEAXPEAX00@Z.generalized"}
+
Index: clang/lib/CodeGen/CodeGenModule.h
===
--- clang/lib/CodeGen/CodeGenModule.h
+++ clang/lib/CodeGen/CodeGenModule.h
@@ -1406,6 +1406,10 @@
   void CreateFunctionTypeMetadataForIcall(const FunctionDecl *FD,
   llvm::Function *F);
 
+  /// Create and attach type metadata to the given call.
+  void CreateFunctionTypeMetadataForIcall(const QualType &QT,
+  llvm::CallBase *CB);
+
   /// Whether this function's return type has no side effects, and thus may
   /// be trivially discarded if it is unused.
   bool MayDropFunctionReturn(const ASTContext &Context, QualType ReturnType);
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -1873,8 +1873,9 @@
 
   // In the cross-dso CFI mode with canonical jump tables, we want !type
   // attributes on definitions only.
-  if (CodeGenOpts.SanitizeCfiCrossDso &&
-  CodeGenOpts.SanitizeCfiCanonicalJumpTables) {
+  if ((CodeGenOpts.SanitizeCfiCrossDso &&
+   CodeGenOpts.SanitizeCfiCanonicalJumpTables) ||
+  CodeGenOpts.CallGraphSection) {
 if (auto *FD = dyn_cast(D)) {
   // Skip available_externally functions. They won't be codegen'ed in the
   // current module anyway.
@@ -2057,7 +2058,17 @@
 
 void CodeGenModule::CreateFunctionTypeMetadataForIcall(const FunctionDecl *FD,
llvm::Function *F) {
-  // Only if we are checking indirect calls.
+  bool EmittedMDIdGeneralized = fa

[PATCH] D105911: [CallGraphSection] Introduce CGSectionFuncComdatCreator pass

[Necip Fazil Yildiran via Phabricator via cfe-commits]

necipfazil created this revision.
necipfazil added reviewers: lattner, morehouse, kcc, llvm-commits.
Herald added subscribers: ormris, hiraditya, mgorny.
necipfazil requested review of this revision.
Herald added projects: clang, LLVM.
Herald added a subscriber: cfe-commits.

Create comdats for functions whose symbols will be referenced from the
call graph section. These comdats are used to create the call graph
sections, so that, the sections can get discarded by the linker if the
functions get removed.

Original RFC: https://lists.llvm.org/pipermail/llvm-dev/2021-June/151044.html
Updated RFC: https://lists.llvm.org/pipermail/llvm-dev/2021-July/151739.html


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D105911

Files:
  clang/lib/CodeGen/BackendUtil.cpp
  llvm/include/llvm/Transforms/Utils/CGSectionFuncComdatCreator.h
  llvm/lib/Passes/PassBuilder.cpp
  llvm/lib/Passes/PassRegistry.def
  llvm/lib/Transforms/Utils/CGSectionFuncComdatCreator.cpp
  llvm/lib/Transforms/Utils/CMakeLists.txt
  llvm/test/Transforms/Util/create-function-comdats.ll

Index: llvm/test/Transforms/Util/create-function-comdats.ll
===
--- /dev/null
+++ llvm/test/Transforms/Util/create-function-comdats.ll
@@ -0,0 +1,21 @@
+; Tests that we create function comdats properly and only for those with no
+; comdats.
+
+; RUN: opt -passes='cg-section-func-comdat-creator' -S %s | FileCheck %s
+
+; CHECK: $f = comdat noduplicates
+; CHECK: $g = comdat any
+$g = comdat any
+
+; CHECK: define dso_local void @f() comdat {
+define dso_local void @f() {
+entry:
+  ret void
+}
+
+; CHECK: define dso_local void @g() comdat {
+define dso_local void @g() comdat {
+entry:
+  ret void
+}
+
Index: llvm/lib/Transforms/Utils/CMakeLists.txt
===
--- llvm/lib/Transforms/Utils/CMakeLists.txt
+++ llvm/lib/Transforms/Utils/CMakeLists.txt
@@ -7,6 +7,7 @@
   BreakCriticalEdges.cpp
   BuildLibCalls.cpp
   BypassSlowDivision.cpp
+  CGSectionFuncComdatCreator.cpp
   CallPromotionUtils.cpp
   CallGraphUpdater.cpp
   CanonicalizeAliases.cpp
Index: llvm/lib/Transforms/Utils/CGSectionFuncComdatCreator.cpp
===
--- /dev/null
+++ llvm/lib/Transforms/Utils/CGSectionFuncComdatCreator.cpp
@@ -0,0 +1,98 @@
+//===-- CGSectionFuncComdatCreator.cpp - CG section func comdat creator ---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+// This pass creates comdats for functions whose symbols will be referenced
+// from the call graph section. These comdats are used to create the call graph
+// sections, so that, the sections can get discarded by the linker if the
+// functions get removed.
+//
+//===--===//
+
+#include "llvm/Transforms/Utils/CGSectionFuncComdatCreator.h"
+
+#include "llvm/ADT/Triple.h"
+#include "llvm/IR/Instructions.h"
+
+namespace llvm {
+
+namespace {
+class ModuleCGSectionFuncComdatCreator {
+public:
+  bool instrumentModule(Module &);
+
+private:
+  bool createFunctionComdat(Function &F, const Triple &T) const;
+  bool hasIndirectCalls(const Function &F) const;
+  bool isTargetToIndirectCalls(const Function &F) const;
+};
+
+bool ModuleCGSectionFuncComdatCreator::createFunctionComdat(
+Function &F, const Triple &T) const {
+  if (auto *Comdat = F.getComdat())
+return false;
+  assert(F.hasName());
+  Module *M = F.getParent();
+
+  // Make a new comdat for the function. Use the "no duplicates" selection kind
+  // if the object file format supports it. For COFF we restrict it to non-weak
+  // symbols.
+  Comdat *C = M->getOrInsertComdat(F.getName());
+  if (T.isOSBinFormatELF() || (T.isOSBinFormatCOFF() && !F.isWeakForLinker()))
+C->setSelectionKind(Comdat::NoDuplicates);
+  F.setComdat(C);
+  return true;
+}
+
+bool ModuleCGSectionFuncComdatCreator::hasIndirectCalls(
+const Function &F) const {
+  for (const auto &I : F)
+if (const CallInst *CI = dyn_cast(&I))
+  if (CI->isIndirectCall())
+return true;
+  return false;
+}
+
+bool ModuleCGSectionFuncComdatCreator::isTargetToIndirectCalls(
+const Function &F) const {
+  return !F.hasLocalLinkage() ||
+ F.hasAddressTaken(nullptr,
+   /* IgnoreCallbackUses */ true,
+   /* IgnoreAssumeLikeCalls */ true,
+   /* IgnoreLLVMUsed */ false);
+}
+
+bool ModuleCGSectionFuncComdatCreator::instrumentModule(Module &M) {
+  Triple TargetTriple = Triple(M.getTargetTriple());
+
+  bool CreatedComdats = false;
+
+  for (Function &F : M) {
+if (isTargetToIndirectCalls(F) || hasIndirectCalls(F))

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

[Necip Fazil Yildiran via Phabricator via cfe-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

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Necip Fazil Yildiran via Phabricator via cfe-commits]

necipfazil updated this revision to Diff 358463.
necipfazil added a comment.

rebase, fix nits in docs


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105907

Files:
  clang/docs/CallGraphSection.rst
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  llvm/include/llvm/CodeGen/CommandFlags.h
  llvm/include/llvm/Target/TargetOptions.h
  llvm/lib/CodeGen/CommandFlags.cpp

Index: llvm/lib/CodeGen/CommandFlags.cpp
===
--- llvm/lib/CodeGen/CommandFlags.cpp
+++ llvm/lib/CodeGen/CommandFlags.cpp
@@ -86,6 +86,7 @@
 CGOPT(DebuggerKind, DebuggerTuningOpt)
 CGOPT(bool, EnableStackSizeSection)
 CGOPT(bool, EnableAddrsig)
+CGOPT(bool, EnableCallGraphSection)
 CGOPT(bool, EmitCallSiteInfo)
 CGOPT(bool, EnableMachineFunctionSplitter)
 CGOPT(bool, EnableDebugEntryValues)
@@ -407,6 +408,11 @@
   cl::init(false));
   CGBINDOPT(EnableAddrsig);
 
+  static cl::opt EnableCallGraphSection(
+  "call-graph-section", cl::desc("Emit a call graph section"),
+  cl::init(false));
+  CGBINDOPT(EnableCallGraphSection);
+
   static cl::opt EmitCallSiteInfo(
   "emit-call-site-info",
   cl::desc(
@@ -520,6 +526,7 @@
   Options.EmitStackSizeSection = getEnableStackSizeSection();
   Options.EnableMachineFunctionSplitter = getEnableMachineFunctionSplitter();
   Options.EmitAddrsig = getEnableAddrsig();
+  Options.EmitCallGraphSection = getEnableCallGraphSection();
   Options.EmitCallSiteInfo = getEmitCallSiteInfo();
   Options.EnableDebugEntryValues = getEnableDebugEntryValues();
   Options.PseudoProbeForProfiling = getPseudoProbeForProfiling();
Index: llvm/include/llvm/Target/TargetOptions.h
===
--- llvm/include/llvm/Target/TargetOptions.h
+++ llvm/include/llvm/Target/TargetOptions.h
@@ -127,11 +127,11 @@
   EmulatedTLS(false), ExplicitEmulatedTLS(false), EnableIPRA(false),
   EmitStackSizeSection(false), EnableMachineOutliner(false),
   EnableMachineFunctionSplitter(false), SupportsDefaultOutlining(false),
-  EmitAddrsig(false), EmitCallSiteInfo(false),
-  SupportsDebugEntryValues(false), EnableDebugEntryValues(false),
-  PseudoProbeForProfiling(false), ValueTrackingVariableLocations(false),
-  ForceDwarfFrameSection(false), XRayOmitFunctionIndex(false),
-  DebugStrictDwarf(false),
+  EmitAddrsig(false), EmitCallGraphSection(false),
+  EmitCallSiteInfo(false), SupportsDebugEntryValues(false),
+  EnableDebugEntryValues(false), PseudoProbeForProfiling(false),
+  ValueTrackingVariableLocations(false), ForceDwarfFrameSection(false),
+  XRayOmitFunctionIndex(false), DebugStrictDwarf(false),
   FPDenormalMode(DenormalMode::IEEE, DenormalMode::IEEE) {}
 
 /// DisableFramePointerElim - This returns true if frame pointer elimination
@@ -290,6 +290,9 @@
 /// to selectively generate basic block sections.
 std::shared_ptr BBSectionsFuncListBuf;
 
+/// Emit section containing call graph metadata.
+unsigned EmitCallGraphSection : 1;
+
 /// The flag enables call site info production. It is used only for debug
 /// info, and it is restricted only to optimized code. This can be used for
 /// something else, so that should be controlled in the frontend.
Index: llvm/include/llvm/CodeGen/CommandFlags.h
===
--- llvm/include/llvm/CodeGen/CommandFlags.h
+++ llvm/include/llvm/CodeGen/CommandFlags.h
@@ -122,6 +122,8 @@
 
 bool getEnableAddrsig();
 
+bool getEnableCallGraphSection();
+
 bool getEmitCallSiteInfo();
 
 bool getEnableMachineFunctionSplitter();
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -6413,6 +6413,10 @@
options::OPT_fno_apple_pragma_pack, false))
 CmdArgs.push_back("-fapple-pragma-pack");
 
+  if (Args.hasFlag(options::OPT_fcall_graph_section,
+   options::OPT_fno_call_graph_section, false))
+CmdArgs.push_back("-fcall-graph-section");
+
   if (Args.hasFlag(options::OPT_fxl_pragma_pack,
options::OPT_fno_xl_pragma_pack, RawTriple.isOSAIX()))
 CmdArgs.push_back("-fxl-pragma-pack");
Index: clang/lib/CodeGen/BackendUtil.cpp
===
--- clang/lib/CodeGen/BackendUtil.cpp
+++ clang/lib/CodeGen/BackendUtil.cpp
@@ -568,6 +568,7 @@
   Options.EmitStackSizeSection = CodeGenOpts.StackSizeSection;
   Options.StackUsageOutput = CodeGenOpts.StackUsageOutput;
   Options.EmitAddrsig = CodeGenOpts.Addrsig;
+  Options.EmitCallGraphSection = Code

[PATCH] D105909: [clang][CallGraphSection] Add type id metadata to indirect call and targets

[Necip Fazil Yildiran via Phabricator via cfe-commits]

necipfazil added inline comments.



Comment at: clang/lib/CodeGen/CGObjCMac.cpp:2288
+}
+  }
 

morehouse wrote:
> This is for Objective C?  I don't think we care about Objective C.
I will remove this.



Comment at: clang/lib/CodeGen/CodeGenModule.cpp:1878
+   CodeGenOpts.SanitizeCfiCanonicalJumpTables) ||
+  CodeGenOpts.CallGraphSection) {
 if (auto *FD = dyn_cast(D)) {

morehouse wrote:
> Why do we need the type metadata on function definitions?
Actually, we don’t.  It seems like having this may lead to duplicate metadata 
as well.

I will remove this.



Comment at: clang/test/CodeGen/call-graph-section.c:39
+  fp_qux(0, 0, 0);
+}
+

morehouse wrote:
> Should we also add a test with a C struct parameter?
I will add one.



Comment at: clang/test/CodeGen/call-graph-section.c:41
+
+// ITANIUM-DAG: [[TVOID_GENERALIZED]] = !{{{.*}}!"_ZTSFvE.generalized"}
+// ITANIUM-DAG: [[TPRIMITIVE_GENERALIZED]] = !{{{.*}}!"_ZTSFicfdE.generalized"}

morehouse wrote:
> What is the `{{.*}}` consuming here?
Internally, for functions, this metadata is added using 
`llvm::GlobalObject::addTypeMetadata()`, which takes an offset parameter. This 
is always 0 for our case. `{{.*}}` is consuming `i64 0,` if the metadata 
belongs to a function. Otherwise, for callsites, it consumes nothing.

I will make the distinction in the test case.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105909

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D105909: [clang][CallGraphSection] Add type id metadata to indirect call and targets

[Necip Fazil Yildiran via Phabricator via cfe-commits]

necipfazil updated this revision to Diff 358505.
necipfazil added a comment.

Several review comments are addressed

- Don't emit type metadata for Objective-C
- Don't emit metadata for function definitions, which leads to duplicates
- Improve the test case
  - Add C struct parameter test
  - Fix variable capturing
  - Make function and callsite metadata checking distinct
  - Some formatting

TODO: a C++ test will be added with class functions, templates, etc.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105909

Files:
  clang/lib/CodeGen/CGClass.cpp
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/CodeGen/CGExprCXX.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/CodeGen/CodeGenModule.h
  clang/test/CodeGen/call-graph-section.c

Index: clang/test/CodeGen/call-graph-section.c
===
--- /dev/null
+++ clang/test/CodeGen/call-graph-section.c
@@ -0,0 +1,75 @@
+// Tests that we assign appropriate identifiers to indirect calls and targets.
+
+// RUN: %clang_cc1 -triple x86_64-unknown-linux -fcall-graph-section -S \
+// RUN: -emit-llvm -o - %s | FileCheck --check-prefixes=CHECK,ITANIUM %s
+
+// RUN: %clang_cc1 -triple x86_64-pc-windows-msvc -fcall-graph-section -S \
+// RUN: -emit-llvm -o - %s | FileCheck --check-prefixes=CHECK,MS %s
+
+// CHECK-DAG: define {{(dso_local)?}} void @foo({{.*}} !type [[F_TVOID:![0-9]+]]
+void foo() {
+}
+
+// CHECK-DAG: define {{(dso_local)?}} void @bar({{.*}} !type [[F_TVOID]]
+void bar() {
+  void (*fp)() = foo;
+  // CHECK: call {{.*}} !type [[CS_TVOID:![0-9]+]]
+  fp();
+}
+
+// CHECK-DAG: define {{(dso_local)?}} i32 @baz({{.*}} !type [[F_TPRIMITIVE:![0-9]+]]
+int baz(char a, float b, double c) {
+  return 1;
+}
+
+// CHECK-DAG: define {{(dso_local)?}} i32* @qux({{.*}} !type [[F_TPTR:![0-9]+]]
+int *qux(char *a, float *b, double *c) {
+  return 0;
+}
+
+// CHECK-DAG: define {{(dso_local)?}} void @corge({{.*}} !type [[F_TVOID]]
+void corge() {
+  int (*fp_baz)(char, float, double) = baz;
+  // CHECK: call i32 {{.*}}, !type [[CS_TPRIMITIVE:![0-9]+]]
+  fp_baz('a', .0f, .0);
+
+  int *(*fp_qux)(char *, float *, double *) = qux;
+  // CHECK: call i32* {{.*}}, !type [[CS_TPTR:![0-9]+]]
+  fp_qux(0, 0, 0);
+}
+
+struct st1 {
+  int *(*fp)(char *, float *, double *);
+};
+
+struct st2 {
+  struct st1 m;
+};
+
+// CHECK-DAG: define {{(dso_local)?}} void @stf({{.*}} !type [[F_TVOID]]
+void stf() {
+  struct st1 St1;
+  St1.fp = qux;
+  // CHECK: call i32* {{.*}}, !type [[CS_TPTR]]
+  St1.fp(0, 0, 0);
+
+  struct st2 St2;
+  St2.m.fp = qux;
+  // CHECK: call i32* {{.*}}, !type [[CS_TPTR]]
+  St2.m.fp(0, 0, 0);
+}
+
+// ITANIUM-DAG: [[F_TVOID]] = !{i64 0, !"_ZTSFvE.generalized"}
+// ITANIUM-DAG: [[CS_TVOID]] = !{!"_ZTSFvE.generalized"}
+// MS-DAG: [[F_TVOID]] = !{i64 0, !"?6AX@Z.generalized"}
+// MS-DAG: [[CS_TVOID]] = !{!"?6AX@Z.generalized"}
+
+// ITANIUM-DAG: [[F_TPRIMITIVE]] = !{i64 0, !"_ZTSFicfdE.generalized"}
+// ITANIUM-DAG: [[CS_TPRIMITIVE]] = !{!"_ZTSFicfdE.generalized"}
+// MS-DAG: [[F_TPRIMITIVE]] = !{i64 0, !"?6AHDMN@Z.generalized"}
+// MS-DAG: [[CS_TPRIMITIVE]] = !{!"?6AHDMN@Z.generalized"}
+
+// ITANIUM-DAG: [[F_TPTR]] = !{i64 0, !"_ZTSFPvS_S_S_E.generalized"}
+// ITANIUM-DAG: [[CS_TPTR]] = !{!"_ZTSFPvS_S_S_E.generalized"}
+// MS-DAG: [[F_TPTR]] = !{i64 0, !"?6APEAXPEAX00@Z.generalized"}
+// MS-DAG: [[CS_TPTR]] = !{!"?6APEAXPEAX00@Z.generalized"}
Index: clang/lib/CodeGen/CodeGenModule.h
===
--- clang/lib/CodeGen/CodeGenModule.h
+++ clang/lib/CodeGen/CodeGenModule.h
@@ -1406,6 +1406,10 @@
   void CreateFunctionTypeMetadataForIcall(const FunctionDecl *FD,
   llvm::Function *F);
 
+  /// Create and attach type metadata to the given call.
+  void CreateFunctionTypeMetadataForIcall(const QualType &QT,
+  llvm::CallBase *CB);
+
   /// Whether this function's return type has no side effects, and thus may
   /// be trivially discarded if it is unused.
   bool MayDropFunctionReturn(const ASTContext &Context, QualType ReturnType);
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -2057,7 +2057,17 @@
 
 void CodeGenModule::CreateFunctionTypeMetadataForIcall(const FunctionDecl *FD,
llvm::Function *F) {
-  // Only if we are checking indirect calls.
+  bool EmittedMDIdGeneralized = false;
+  if (CodeGenOpts.CallGraphSection &&
+  (!F->hasLocalLinkage() ||
+   F->getFunction().hasAddressTaken(nullptr, /* IgnoreCallbackUses */ true,
+/* IgnoreAssumeLikeCalls */ true,
+/* IgnoreLLVMUsed */ false))) {
+F->addTypeMetada