[llvm] [ThinLTO] Populate declaration import status except for distributed ThinLTO (PR #88024)

Mingming Liu via llvm-commits llvm-commits at lists.llvm.org
Fri May 10 17:13:12 PDT 2024


minglotus-6 wrote:

> > #87600 is a functional change and the diffbase of this patch, and `llvm/test/ThinLTO/X86/import_callee_declaration.ll` should be a test case for both patches.
> > In the [diffbase](https://github.com/llvm/llvm-project/pull/87600), bitcode writer takes maps as additional parameters to populate import status, and it's not straightforward to construct regression tests there without this patch. I wonder if I shall introduce `cl::list<std::string>` in llvm-lto/llvm-lto2 (as a repeated arg) to specify `filename:GUID` to test the diffbase alone.
> 
> Rather than add an option just for testing that one alone, I have a suggestion for splitting up the PRs slightly differently. What if you submitted this one first, minus the modified calls to writeIndexToFile and the part of the test that checks the disassembled index (just have the testing for this one check the number of declarations imported and other debug messages). Then move the modified calls to writeIndexToFile and the index disassembly checking to PR87600 that can be committed as a follow on? That way each change comes with a test.

This way of splitting makes a lot of sense. For in-process ThinLTO, `function-import` pass in the backend can see the`declaration` status (i.e., without updating bitcode writer), so I updated `import_callee_declaration.ll` test to test the debugging logs are expected.

https://github.com/llvm/llvm-project/pull/88024


More information about the llvm-commits mailing list