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

Bill Wendling via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Jul 18 16:59:31 PDT 2019


void marked an inline comment as done.
void added inline comments.


================
Comment at: lib/CodeGen/AsmPrinter/AsmPrinterInlineAsm.cpp:436
+              for (const MachineBasicBlock &MBB : *MF)
+                if (BB == MBB.getBasicBlock()) {
+                  MBB.getSymbol()->print(OS, AP->MAI);
----------------
nickdesaulniers wrote:
> I'm not sure that either side of this `if` are being tested/covered by your new test in callbr-asm-obj-file.ll.
> 
> In particular, the case that worries me is passing the address of a label as an input to `asm goto`.  Consider:
> 
> ```
> asm goto(".quad %l0\n\t.quad %l1" :: "i"(&&baz) :: bar);
>         bar:;
>         baz:;
> ```
> 
> https://godbolt.org/z/lKa_HD
> 
> where 2 blockaddresses are parameters to a callbr, one is the address of a label, one is the jump target list.
> 
> I'd be more comfortable signing off on code review if that case was also tested here (and considered in further changes related to `callbr`)
We need to get clang to accept your code snippet first. :-)

I'm not sure I understand what you're saying here. What are you worried will be the outcome?

Despite the looping and whatnot, what this change does is fairly straightforward: instead of getting the IR basic block and creating a label for it, it uses the machine basic block's label directly. Similar to the code on line 436-437.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D64888





More information about the llvm-commits mailing list