[PATCH] D99487: [CodeGen] Port basic block sections from ELF to COFF

[TaoPan via Phabricator via llvm-commits]

TaoPan added a comment.

In D99487#2727370 <https://reviews.llvm.org/D99487#2727370>, @modimo wrote:

> In D99487#2724706 <https://reviews.llvm.org/D99487#2724706>, @tmsriram wrote:
>
>> In D99487#2724702 <https://reviews.llvm.org/D99487#2724702>, @modimo wrote:
>>
>>> Agree with @MaskRay I'd like to see a holistics plan/RFC on all the pieces that are needed to get this functionality. The functionality here is only tested when exceptions are actually propagated which is not common in LLVM test suite. You'll want to run and likely write very specific tests stressing the test matrix here of BBS + MFS + EH. Given all that, a larger plan that details how we can be confident that all the pieces have been accounted for and will be well tested is (IMO) imperative in getting this change reviewed and accepted.
>>
>> Since this (the test matrix) is already done for ELF, would it be complete if the author added equivalent tests for COFF?
>
> Good point, that sounds like a good avenue to pursue. @snehasish mentions some of these exact tests here https://reviews.llvm.org/D95209#2523227. Looks like there was some movement for these tests here: https://reviews.llvm.org/D96393 although it appears BBS might need Windows debuginfo support for testing. I'm chasing through a bunch of these diffs to figure out what the state of this is and I'm not quite certain what it is. @TaoPan do you have a plan you can share about the progress here so we can follow along?

Sorry for confusion! Let me summarize the history.

1. I submitted D95209 <https://reviews.llvm.org/D95209> for porting MFS to Windows COFF. @tmsriram and @snehasish mentioned CFI, DebugInfo and exception handling should be guaranteed, thanks for their help and guidance!
2. I submitted D96393 <https://reviews.llvm.org/D96393> for adding MFS DebugInfo test, as MFS COFF implementation code is in 1, it includes only Linux ELF test. I planed to extend MFS DebugInfo COFF test in 1 at that time. @snehasish guided me that add (MFS dependent) BBS not MFS DebugInfo test on Windows COFF, just like Linux ELF.
3. I submitted this patch D99487 <https://reviews.llvm.org/D99487> for supporting basic function of BBS on Windows COFF.
4. I submitted D100735 <https://reviews.llvm.org/D100735> for making sure Windows exception handling for BBS. As Linux ELF exception handling test llvm/test/CodeGen/X86/basic-block-sections-eh.ll is mainly testing throw & catch exception, so I referred https://docs.microsoft.com/en-us/cpp/cpp/structured-exception-handling-c-cpp?view=msvc-160 to generate ll test file.
5. I submitted D101421 <https://reviews.llvm.org/D101421> for making sure Windows CodeView DebugInfo for BBS. I used same C code of llvm/test/CodeGen/X86/cfi-basic-block-sections-1.ll introduced by D79978 <https://reviews.llvm.org/D79978> that is CFI DebugInfo for BBS on Linux ELF to generate ll test file.

So the dependence chain is: 1 depends on 3 depends on 4 and 5. 2 will be abandoned.
Could you please review the upper progress? Any question is welcome.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99487