[PATCH] D79379: Don't add function to import list if it's defined in the same module

Hongtao Yu via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue May 5 23:57:37 PDT 2020


hoyFB added inline comments.


================
Comment at: llvm/include/llvm/ProfileData/SampleProf.h:531
+    auto isDeclaration = [](const Function *F) {
+      return !F || F->isDeclaration();
+    };
----------------
Should `isDeclarationForLinker` be checked instead? I thought when a function is imported, it has definition in the current module and it's not longer a declaration. An imported function is a definition in the module it's imported into but since it is also a `isDeclarationForLinker` (or `hasAvailableExternallyLinkage`) it won't be emitted into the current object file.


================
Comment at: llvm/test/Transforms/SampleProfile/function_metadata.ll:57
 !10 = !{!"clang version 3.5 "}
 !11 = distinct !DISubprogram(name: "foo_available", line: 7, isLocal: false, isDefinition: true, virtualIndex: 6, flags: DIFlagPrototyped, isOptimized: false, unit: !0, scopeLine: 7, file: !1, scope: !1, type: !6, retainedNodes: !2)
 !12 = distinct !DISubprogram(name: "test_liveness", line: 7, isLocal: false, isDefinition: true, virtualIndex: 6, flags: DIFlagPrototyped, isOptimized: false, unit: !0, scopeLine: 7, file: !1, scope: !1, type: !6, retainedNodes: !2)
----------------
Nit: remove `!11` or replace it with `!13`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79379





More information about the llvm-commits mailing list