[PATCH] D64888: Use the MachineBasicBlock symbol for a callbr target

[Nick Desaulniers via Phabricator via llvm-commits]

nickdesaulniers added inline comments.


================
Comment at: llvm/trunk/lib/CodeGen/AsmPrinter/AsmPrinterInlineAsm.cpp:434
+              const BasicBlock *BB = BA->getBasicBlock();
+              const MachineFunction *MF = MI->getParent()->getParent();
+              for (const MachineBasicBlock &MBB : *MF)
----------------
So another issue I though of with this approach:

It makes sense to me as implemented; we need some mapping from `BlockAddress` of the `BasicBlock` to the corresponding `MachineBasicBlock` (we have the reverse mapping; given a `MachineBasicBlock` we can get the `BasicBlock` it comes from, but we don't have the reverse mapping; `GetBlockAddressSymbol` kind of does this but has other issues).  This approach works for the test case provided, and probably all C/C++  that could ever hit this path BUT it is not safe to assume that the `MachineBasicBlock` corresponding to the `BlockAddress` is within this `MachineFunction`.

Stated another way, I could have `blockaddress(@foo, %bar)` used in `MachineFunction` `@baz`. The code in this patch would iterate the `MachineBasicBlocks` of the `MachineFunction` corresponding to `@baz` since the `MachineInstruction` using the `blockaddress` constant was in `@baz`.  It would never find `%bar` since it needs to iterate `MachineBasicBlocks` of `@foo`, not `@baz`.

I think this would be fixed by initializing `MF` to `BB->getParent()`.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D64888