[PATCH] D154294: [Windows SEH]: Do not fold branches for MSVC TableSEH function

Sat Jul 15 20:49:14 PDT 2023

pengfei added a subscriber: majnemer.
pengfei added a comment.

In D154294#4503523 <https://reviews.llvm.org/D154294#4503523>, @efriedma wrote:

> In D154294#4503150 <https://reviews.llvm.org/D154294#4503150>, @pengfei wrote:
>
>>> I'm concerned that if we ignore the bad CFG now, it's going to come back to bite us again.
>>
>> It doesn't look to me there's a bad CFG if we never do BranchFolding here. You can find the predecessors/successors chain EH BBs correctly in the CFG. The problem looks to me is BranchFolding ignores EH edges by itself.
>
> If you don't do BranchFolding, the CFG is... sort-of right?  I mean, disabling BranchFolding and any similar pass (I'm drawing a blank off the top of my head, but I'm pretty sure BranchFolding is not the only pass that does relevant CFG manipulation), the CFG continues to resemble the IR CFG; you can use that to compute the correct EH edges, and stuff like MachineDominatorTree will work correctly within the EH blocks.
>
> Even with that, though, without the complete set of proper edges, vreg liveness is going to be wrong.  Probably not relevant in a lot of cases because clang uses "volatile" heavily, and I think the SEH landing pad clobbers every physical register.  But I suspect it's not completely irrelevant.
>
> Also, this patch isn't implementing "disable BranchFolding" in a good way.  Checking "getPersonalityFn" explicitly instead of marking up the MIR means that we potentially need to copy-paste the getPersonalityFn check into any pass that does CFG modifications.  And people making changes to those passes will, inevitably, not be aware they need to copy-paste it.

I'm not sure of that, but I got some evidence that BranchFolding is special. You can see `EHScopeMembership` is designed for BranchFolding use only. I also found two related patches rG161935520d5a9 <https://reviews.llvm.org/rG161935520d5a9cd1fcaddee39bb8438bcfec5552> and rG4600c064346 <https://reviews.llvm.org/rG4600c0643466114c20a557b74b233665105cf9bb> by @majnemer. Though they are targeting to funclets, I think the problem is similar, i.e., there're no edges to funclets so we have to make CFG irreducible manually.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154294