[PATCH] D76961: [BranchFolder] don't remove MBB's that have their address taken
Nick Desaulniers via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Mon Mar 30 10:50:10 PDT 2020
nickdesaulniers planned changes to this revision.
nickdesaulniers added a comment.
> An INLINEASM_BR edge should count as a predecessor, yes; if that edge is missing, something went wrong.
Ok, that's what I suspected (should be no difference for direct vs indirect references), thanks for confirming.
I'm going to sit on this change; the test cases are helpful and expose a current issue, but the fix feels like more duct tape and bailing wire to me, and I'm not certain it's the right fix.
>From here, my plan is:
1. implement separate machine verifier checks for:
1. all `MachineBasicBlocks` with `INLINEASM_BR` terminators should have the targets of the `INLINEASM_BR` in their list of successors.
2. all `MachineBasicBlocks` that are targets of `INLINEASM_BR` should have the `INLINEASM_BR`'s parent `MachineBasicBlock` in their list of predacessors. This is the invariant that's violated in the above test case.
2. fix whatever currently unidentified pass is breaking those invariants and fix them.
3. revisit landing this test case, maybe without the changes I've made here to `BranchFolding`.
(Then landing in the order 2, 1, 3).
I had to beef up the verification of `callbr` at the `Instruction` level to help find previous breakages, so this feels like a similar plan, albeit at a lower IR. I'll cc folks when I have those ready, will be my focus this week.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D76961/new/
https://reviews.llvm.org/D76961
More information about the llvm-commits
mailing list