[PATCH] D100735: [CodeGen] Enable Windows exception handling for basic block sections

[TaoPan via Phabricator via llvm-commits]

TaoPan added a comment.

In D100735#2842162 <https://reviews.llvm.org/D100735#2842162>, @modimo wrote:

> In D100735#2838154 <https://reviews.llvm.org/D100735#2838154>, @TaoPan wrote:
>
>> I tried to build with clang, define it as "main" and add printf to the __except clause
>>
>>   without BBS, the exception can be captured, printf log in __except clause was printed.
>>   with BBS,  build fail with "error: Cannot represent this expression", I'm investigating this error. 
>
> Cool, feel free to file a bug and link it here to track progress.

This bug depends on #D99487 and this commit, can't be reproduced without these two commits. Is it ok file the bug after landing these two commits?

>> I don't know the reason MBB->setIsEHPad() without any MI of the MBB setIsEHLabel() in the case of Windows COFF.
>
> My cursory understanding is that Windows COFF in LLVM uses a different set of intrinsics to represent EH which probably don't fall under EHLabel. There's probably a good reason for that. If this is the case then probably better to guard it like you're currently doing from scanning wastefully. I would guard with `if (!<triple for windows>)` rather specifically checking for ELF. Also, put the check inside `avoidZeroOffsetLandingPad` rather than the call to it so the call site doesn't have to be aware of this.

Thanks for your analysis! I changed code according to your guidance.



================
Comment at: llvm/test/MC/COFF/seh-bbs.ll:88
+; BBS-NEXT:            0x18 IMAGE_REL_AMD64_ADDR32NB .text
+; BBS-NEXT:            0x20 IMAGE_REL_AMD64_ADDR32NB ?TestCPPEX@@YAXH at Z.__part.3
+; BBS-NEXT:          ]
----------------
modimo wrote:
> I would interleave these with the baseline so its easy to see how exactly the codegen differs, for this example:
> 
> ```
> FileCheck --check-prefix=COMMON,BASELINE %s
> FileCheck --check-prefix=COMMON,BBS %s
> ...
> ; COMMON:          Relocations [
> ; COMMON-NEXT:       0xC IMAGE_REL_AMD64_ADDR32NB __C_specific_handler
> ; COMMON-NEXT:       0x14 IMAGE_REL_AMD64_ADDR32NB .text
> ; COMMON-NEXT:       0x18 IMAGE_REL_AMD64_ADDR32NB .text
> ; BASELINE:       0x20 IMAGE_REL_AMD64_ADDR32NB .text
> ; BBS:            0x20 IMAGE_REL_AMD64_ADDR32NB ?TestCPPEX@@YAXH at Z.__part.3
> ```
> 
> It's also good to add short comments for these diff spots on why this is expected behavior.
Good idea! I changed check format as you guided.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100735