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

[Xu Mingjie via Phabricator via llvm-commits]

Enna1 added a comment.

In D115417#3185707 <https://reviews.llvm.org/D115417#3185707>, @tejohnson wrote:

> 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.

I'm glad to send another patch to add an option that defines PreOptModuleHook or PreOptModuleHook for testing in llvm-lto2 :)

>> 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.

I'm sorry I forget to mention, I tried adding this bug case in lld/test/ELF/lto/opt-remarks.ll,  but it not works.
Yes, the emit-llvm is what triggers the addition of the hooks, it also need Regular LTO detects some dead functions.
In lld/test/ELF/lto/opt-remarks.ll Regular LTO will not detected any dead function, and there is no optimization remarks for removed functions, so we can not simply add a case in lld/test/ELF/lto/opt-remarks.ll to test this bug case.

We can specify a symbol resolution using `llvm-lto2`,  so we can make Regular LTO detect dead functions and emit optimization remarks for removed functions, when testing this bug via `llvm-lto2`.
But as I mentioned above, `llvm-lto2` does not have an option that defines PreOptModuleHook or PreOptModuleHook, so `llvm-lto2` can not trigger the addition of the hooks.

Thanks!

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