[llvm-branch-commits] [llvm] [ThinLTO] Generate import status in per-module combined summary (PR #88024)

[Jan Voung via llvm-branch-commits]

@@ -276,7 +276,7 @@ class ThinLTOCodeGenerator {
   void gatherImportedSummariesForModule(
   Module &Module, ModuleSummaryIndex &Index,
   std::map &ModuleToSummariesForIndex,
-  const lto::InputFile &File);
+  const lto::InputFile &File, GVSummaryPtrSet &DecSummaries);

jvoung wrote:

nit: consider keeping the outparams together (so DecSummaries next to 
ModuleToSummariesForIndex, and File after?)

https://github.com/llvm/llvm-project/pull/88024
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [llvm] [ThinLTO] Generate import status in per-module combined summary (PR #88024)

[Jan Voung via llvm-branch-commits]

@@ -833,9 +833,14 @@ void ThinLTOCodeGenerator::emitImports(Module &TheModule, 
StringRef OutputName,
ExportLists);
 
   std::map ModuleToSummariesForIndex;
+  // 'EmitImportsFiles' emits the list of modules from which to import from, 
and
+  // the set of keys in `ModuleToSummariesForIndex` should be a superset of 
keys
+  // in `ModuleToDecSummaries`, so no need to use `ModuleToDecSummaries` in

jvoung wrote:

nit: update DecSummaries in comment

https://github.com/llvm/llvm-project/pull/88024
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [llvm] [ThinLTO] Generate import status in per-module combined summary (PR #88024)

[Jan Voung via llvm-branch-commits]

@@ -207,11 +211,16 @@ bool convertToDeclaration(GlobalValue &GV);
 /// \p ModuleToSummariesForIndex will be populated with the needed summaries
 /// from each required module path. Use a std::map instead of StringMap to get
 /// stable order for bitcode emission.
+///
+/// \p ModuleToDecSummaries will be populated with the set of declarations \p
+/// ModulePath need from other modules. They key is module path, and the value

jvoung wrote:

nit: ModuleToDecSummaries -> DecSummaries now
and update the "The key is ..." part

https://github.com/llvm/llvm-project/pull/88024
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [llvm] [ThinLTO][Bitcode] Generate import type in bitcode (PR #87600)

[Jan Voung via llvm-branch-commits]

@@ -216,7 +216,8 @@ void gatherImportedSummariesForModule(
 StringRef ModulePath,
 const DenseMap &ModuleToDefinedGVSummaries,
 const FunctionImporter::ImportMapTy &ImportList,
-std::map &ModuleToSummariesForIndex);
+std::map &ModuleToSummariesForIndex,
+GVSummaryPtrSet &DecSummaries);

jvoung wrote:

Can you add a documentation for the DecSummaries parameter here too? (similar 
to the ThinLTOCodeGenerator.h comment, might be useful to indicate it's a 
subset of ModuleToSummariesForIndex).

https://github.com/llvm/llvm-project/pull/87600
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [llvm] [ThinLTO][Bitcode] Generate import type in bitcode (PR #87600)

[Jan Voung via llvm-branch-commits]

https://github.com/jvoung edited https://github.com/llvm/llvm-project/pull/87600
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [llvm] [ThinLTO][Bitcode] Generate import type in bitcode (PR #87600)

[Jan Voung via llvm-branch-commits]

@@ -1399,18 +1399,21 @@ class lto::ThinBackendProc {
   llvm::StringRef ModulePath,
   const std::string &NewModulePath) {
 std::map ModuleToSummariesForIndex;
+GVSummaryPtrSet DeclarationSummaries;
 
 std::error_code EC;
 gatherImportedSummariesForModule(ModulePath, ModuleToDefinedGVSummaries,
- ImportList, ModuleToSummariesForIndex);
+ ImportList, ModuleToSummariesForIndex,
+ DeclarationSummaries);
 
 raw_fd_ostream OS(NewModulePath + ".thinlto.bc", EC,
   sys::fs::OpenFlags::OF_None);
 if (EC)
   return errorCodeToError(EC);
 
 // TODO: Serialize declaration bits to bitcode.

jvoung wrote:

Can the TODO be removed?

https://github.com/llvm/llvm-project/pull/87600
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [llvm] [ThinLTO][Bitcode] Generate import type in bitcode (PR #87600)

[Jan Voung via llvm-branch-commits]

https://github.com/jvoung commented:

Nice!

https://github.com/llvm/llvm-project/pull/87600
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [llvm] [ThinLTO][Bitcode] Generate import type in bitcode (PR #87600)

[Jan Voung via llvm-branch-commits]

@@ -48,20 +48,20 @@
 ; First disassemble per-module summary and find out the GUID for {large_func, 
large_indirect_callee}.
 ;
 ; RUN: llvm-dis lib.bc -o - | FileCheck %s --check-prefix=LIB-DIS
+; LIB-DIS: [[LIBMOD:\^[0-9]+]] = module: (path: "lib.bc", hash: (0, 0, 0, 0, 
0))
 ; LIB-DIS: [[LARGEFUNC:\^[0-9]+]] = gv: (name: "large_func", summaries: 
{{.*}}) ; guid = 2418497564662708935
 ; LIB-DIS: [[LARGEINDIRECT:\^[0-9]+]] = gv: (name: "large_indirect_callee", 
summaries: {{.*}}) ; guid = 14343440786664691134
 ; LIB-DIS: [[LARGEINDIRECTALIAS:\^[0-9]+]] = gv: (name: 
"large_indirect_callee_alias", summaries: {{.*}}, aliasee: [[LARGEINDIRECT]]
 ;
-; Secondly disassemble main's combined summary and test that large callees are
-; not imported as declarations yet.
-; TODO: Serialize declaration bit and test declaration bits are correctly set.
+; Secondly disassemble main's combined summary and verify the import type of
+; these two GUIDs are declaration.
 ;
 ; RUN: llvm-dis main.bc.thinlto.bc -o - | FileCheck %s --check-prefix=MAIN-DIS
 ;
 ; MAIN-DIS: [[LIBMOD:\^[0-9]+]] = module: (path: "lib.bc", hash: (0, 0, 0, 0, 
0))
-; MAIN-DIS-NOT: [[LARGEFUNC:\^[0-9]+]] = gv: (guid: 2418497564662708935, 
summaries: (function: (module: [[LIBMOD]], flags: ({{.*}} importType: 
declaration), insts: 8, {{.*}})))
-; MAIN-DIS-NOT: [[LARGEINDIRECT:\^[0-9]+]] = gv: (guid: 14343440786664691134, 
summaries: (function: (module: [[LIBMOD]], flags: ({{.*}} importType: 
declaration), insts: 8, {{.*}})))
-; MAIN-DIS-NOT: [[LARGEINDIRECTALIAS:\^[0-9]+]] = gv: (guid: 
16730173943625350469, summaries: (alias: (module: [[LIBMOD]], flags: ({{.*}} 
importType: declaration)
+; MAIN-DIS: [[LARGEFUNC:\^[0-9]+]] = gv: (guid: 2418497564662708935, 
summaries: (function: (module: [[LIBMOD]], flags: ({{.*}} importType: 
declaration), insts: 8, {{.*}})))

jvoung wrote:

It might be interesting to test happens if "calleeAddrs" had just an alias of a 
function and not the original function (right now both large_indirect_callee 
and large_indirect_callee_alias are there?), but could be okay to test that 
later too.

https://github.com/llvm/llvm-project/pull/87600
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits