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

Tue Apr 13 19:06:51 PDT 2021

tmsriram added a comment.

In D99487#2670028 <https://reviews.llvm.org/D99487#2670028>, @TaoPan wrote:

> Hi tmsriram, MaskRay,
>
> I updated implementation and test. Could you please have a look?
> As for multiple COMDAT seletion kinds of PE-COFF, is IMAGE_COMDAT_SELECT_NODUPLICATES wrong? Or need to set different COMDAT selection kind in some case?

I am not familiar with PE-COFF and I don't know  the answer.  FOR ELF, the basic block COMDAT type should be the same as its parent function and should also be in the same group.

> As for a holistic plan how every component is going to be implemented, I'm developing
>
> 1. Basic block sections Windows exception handling, I'm not sure whether need to handle exception handling sections like ELF sections ".eh.*" here, anyway, I'll make sure Windows exception handling work on basic block sections.

If it is easier for you to break this up, you can present exception handling in a separate patch with a clear "TODO:" at the appropriate places. That should be alright and is also easier to review.

> 2. MFS on Windows, I submitted D95209 <https://reviews.llvm.org/D95209>, it includes text section with prefix ".text$split" handling, it will be updated after landing this patch.
>
> in turn.

Great, please use the parent-child relationships to mark these patches.

- Could you also please expand the summary of this patch?  I would write what extra was needed to port this from ELF to PE-COFF.
- Could you also talk about DebugInfo handling and CFI with PE-COFF and basic block sections?  Are these format agnostic and have you verified that they do the right thing? These were the biggest challenges for us when doing this for ELF.

Appreciate your efforts towards porting this to PE-COFF and making it more generally available. Thanks!

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99487