[PATCH] D115417: [LTO] Fix incomplete optimization remarks when PreOptModuleHook or PostInternalizeModuleHook is defined

[Teresa Johnson via Phabricator via llvm-commits]

tejohnson added a comment.

In D115417#3184794 <https://reviews.llvm.org/D115417#3184794>, @Enna1 wrote:

> In https://reviews.llvm.org/D46376, we add support for optimization remarks to ThinLTO Backend, we call `finalizeOptimizationRemarks()` when PreOptModuleHook, PostPromoteModuleHook, PostInternalizeModuleHook, PostImportModuleHook is defined.
>
> This patch call `finalizeOptimizationRemarks()` when PreOptModuleHook or PostInternalizeModuleHook is defined for Regular LTO Backend.
>
> Cause `llvm-lto2` does not have an option to define PreOptModuleHook or PreOptModuleHook for Regular LTO Backend, it's hard to write a testcase.

Ah, that would be a good thing to add support for testing in llvm-lto2, but not needed for this patch I think, see below.

> We can reproduce this bug using the following testcase:
>
> $ clang++ -c -flto tu1.cpp -o tu1.o
> $ clang++ -c -flto tu2.cpp -o tu2.o
> $ clang++ -fuse-ld=lld -flto -foptimization-record-passes=lto tu1.o tu2.o -Wl,--opt-remarks-filename,opt.lto.yaml -Wl,--plugin-opt=emit-llvm

You can just add a test via lld. I.e. see lld/test/ELF/lto/opt-remarks.ll. Can you add this case there? I believe the emit-llvm is what triggers the addition of the hooks, and that is not case being tested yet in that test.

> The content of tu1.cpp and tu2.cpp:
>
>   // tu1.cpp
>   int unused(int a);
>   int probably_inlined(int a);
>   int main(int argc, const char *argv[]) {
>     return probably_inlined(argc);
>   }
>   // tu2.cpp
>   int unused(int a) {
>     return a + 1;
>   }
>   int probably_inlined(int a) {
>     return a + 2;
>   }
>
> The content of `opt.lto.yaml` is empty or incomplete due to this bug.
>
> If this patch is applied, the content of `opt.lto.yaml` will be:
>
>   --- !Passed
>   Pass:            lto
>   Name:            deadfunction
>   Function:        _Z6unusedi
>   Args:
>     - Function:        _Z6unusedi
>     - String:          ' not added to the combined module '
>   ...




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115417