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

[Matt Morehouse via Phabricator via llvm-commits]

morehouse added a comment.

I think we want a C++ test as well, with class functions, templates, etc.

Also, please rebase the patch onto any parent commits, so that it builds properly and the clang-tidy warnings go away.



================
Comment at: clang/lib/CodeGen/CGObjCMac.cpp:2288
+    }
+  }
 
----------------
This is for Objective C?  I don't think we care about Objective C.


================
Comment at: clang/lib/CodeGen/CodeGenModule.cpp:1878
+       CodeGenOpts.SanitizeCfiCanonicalJumpTables) ||
+      CodeGenOpts.CallGraphSection) {
     if (auto *FD = dyn_cast<FunctionDecl>(D)) {
----------------
Why do we need the type metadata on function definitions?


================
Comment at: clang/test/CodeGen/call-graph-section.c:4-7
+// 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
----------------



================
Comment at: clang/test/CodeGen/call-graph-section.c:9
+
+// CHECK: define {{(dso_local )?}}void @foo({{.*}} !type [[TVOID_GENERALIZED:![0-9]+]]
+void foo() {
----------------
Nit:  I find this spacing more readable


================
Comment at: clang/test/CodeGen/call-graph-section.c:13
+
+// CHECK: define {{(dso_local )?}}void @bar({{.*}} !type [[TVOID_GENERALIZED:![0-9]+]]
+void bar() {
----------------
This recaptures the `TVOID_GENERALIZED` variable.  We should reuse the previous one instead: `[[TVOID_GENERALIZED]]`.

Same for other variables below.


================
Comment at: clang/test/CodeGen/call-graph-section.c:39
+  fp_qux(0, 0, 0);
+}
+
----------------
Should we also add a test with a C struct parameter?


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


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105909