[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