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

[Di Mo via Phabricator via llvm-commits]

modimo added a comment.

In D100735#2838154 <https://reviews.llvm.org/D100735#2838154>, @TaoPan wrote:

> I added baseline (without BBS) case and check the size of the Sections table.

Nice thanks. One comment on the format.

> 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.

> The iterator is rolling loop, begin() is next to end(), the '++' operation change the iterator in loop turn, e.g. if size() is 2, the turn of '++' operation is:
>
>   begin() (1st node) -> 2nd node -> end() -> begin() (1st node) -> 2nd node -> ....

Ah it becomes infinite, thanks for clarifying.

> 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.



================
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:          ]
----------------
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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100735