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

Eli Friedman via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Sat Jul 15 11:09:47 PDT 2023


efriedma added a comment.

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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154294



More information about the llvm-commits mailing list