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

Sriraman Tallam via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Apr 14 17:04:10 PDT 2021


tmsriram added a comment.

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

> In D99487#2687506 <https://reviews.llvm.org/D99487#2687506>, @tmsriram wrote:
>
>> 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.
>
> I added "TODO:" for constructing exception section and cold section later.
>
>> Great, please use the parent-child relationships to mark these patches.
>
> I marked parent-child relationships.
>
>> - 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.
>
> I expanded the summary, could you please have a  review?
>
>> - 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.
>
> No, I didn't verify DebugInfo handling and CFI with PE-COFF and basic block sections.

This is quite important to validate the correctness of the generated assembly.  If you could please check that CFI and DebugInfo are sane before we take this patch forward?  Let's get a temperature on how good these are before we decide how to take it forward.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99487



More information about the llvm-commits mailing list