[PATCH] D104431: [lld] Add lto-cspgo-warn-mismatch option for COFF

[Teresa Johnson via Phabricator via llvm-commits]

tejohnson added a comment.

In D104431#2843631 <https://reviews.llvm.org/D104431#2843631>, @YolandaCY wrote:

> In D104431#2836028 <https://reviews.llvm.org/D104431#2836028>, @tejohnson wrote:
>
>> In D104431#2835530 <https://reviews.llvm.org/D104431#2835530>, @YolandaCY wrote:
>>
>>> Thanks Teresa for the comments.
>>>
>>> For the first question, clang uses the -Wno-backend-plugin option to ignore the warnings.
>>> This is mentioned in the How to build clang and LLVM with PGO <https://llvm.org/docs/HowToBuildWithPGO.html> user guide.
>>>
>>> I haven't figured out how this correlates to the internal -no-pgo-warn-mismatch option though. Any tips?
>>
>> I traced it through and it looks like it will disable any warning coming from outside of clang (so LLVM) that doesn't have another type of handling in this switch statement:
>> http://llvm-cs.pcc.me.uk/tools/clang/lib/CodeGen/CodeGenAction.cpp#818
>>
>> So it looks like a larger hammer because it could affect other warning types too.
>>
>> @xur @davidxl do either of you know why we don't support something more fine grained in clang such as gcc's -Wno-coverage-mismatch?
>
> Thanks Teresa for the pointer.
>
> The corresponding lld diagnostic handler can be found at:
> http://llvm-cs.pcc.me.uk/tools/lld/Common/ErrorHandler.cpp#66
> It is much simpler and does not distingush the kind. And it is initialized in the LTOLLVMContext:
> http://llvm-cs.pcc.me.uk/include/llvm/LTO/Config.h#228
>
> It seems that to add a similar option as clang we may need to add an lto specific DiagnosticHandlerFunction (not share lld's) and check for all diagnostic kinds as clang?
> Or shall we just add a fine grained option similar to gcc?

Personally I find it reasonable to have a way to disable just these warnings. I do find it unfortunate that the support for disabling these warnings is and will still be at a different granularity in clang vs LTO.

@xur @davidxl ping on history for not supporting a finer grain option for disabling this warning via clang.

My feeling is that it would be fine to go ahead here but I'd prefer that you add support for the ELF side as well.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104431