[PATCH] D130290: [MachineVerifier] add checks for INLINEASM_BR

[Nick Desaulniers via Phabricator via llvm-commits]

nickdesaulniers added inline comments.


================
Comment at: llvm/lib/CodeGen/MachineVerifier.cpp:882
+
+      const BasicBlock *IndirectTarget = MO.getBlockAddress()->getBasicBlock();
+
----------------
nickdesaulniers wrote:
> arsenm wrote:
> > Why is this based on the IR block?
> Because AFAIK, there is no corresponding `BlockAddress` `Constant` in the MIR level, so the `MachineInst` uses a `BlockAddress` `Constant` which is just a tuple of (`Function`, `BasicBlock`). What would be helpful here would be if `BlockAddress` `Constants` were lowered to something new (doesn't exist, yet) that is a tuple of (`MachineFunction`, `MachineBasicBlock`).
> 
> The existing design seems brittle, because IIUC, there's a one-to-many relationship between `BasicBlock` and `MachineBasicBlock`. A `MachineBasicBlock` can refer to a single `BasicBlock`.
> 
> Perhaps that's why in my TODO below that we _don't_ have a mapping of `BasicBlock` to `MachineBasicBlock`; because they are not one-to-one (unless I'm wrong).
`MachineOperand::getMBB()` is almost what I want here, but that corresponds to a different `MachineOperandType` (`MO_MachineBasicBlock`) than what is being used for `INLINEASM_BR` (`MO_BlockAddress`).  Maybe `MO_BlockAddress` needs to die?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130290