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<FunctionDecl>(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

Reply via email to