[PATCH] D124697: Distinguish between different forms of "address-taken" MachineBasicBlocks

[Eli Friedman via Phabricator via llvm-commits]

efriedma added inline comments.


================
Comment at: llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp:3481
 
-    // MBBs can have their address taken as part of CodeGen without having
-    // their corresponding BB's address taken in IR
-    if (BB && BB->hasAddressTaken())
-      for (MCSymbol *Sym : getAddrLabelSymbolToEmit(BB))
-        OutStreamer->emitLabel(Sym);
+    assert(BB && BB->hasAddressTaken() && "Missing BB");
+    for (MCSymbol *Sym : getAddrLabelSymbolToEmit(BB))
----------------
arsenm wrote:
> efriedma wrote:
> > arsenm wrote:
> > > I don't like this hard requirement on the IR. CodeGen passes should try to avoid depending on the block being present (In particular llvm-reduce is now stripping out IR block references).
> > I'm not sure what alternative you're suggesting here.  There's a contract based on the BasicBlock: we emit a symbol here, and uses of the BlockAddress constant refer to it.  To emit the correct symbol we need the BasicBlock.  If we skip emitting the symbol here, we miscompile.
> This needs to do something to handle the null block. Can it emit an __unnamed symbol or something like for anonymous functions? If this is going to assert on null the verifier at least has to check it
If we're not concerned about emitting code that actually works, we can skip emitting the symbol.  The blockaddress will point to the end of the function.

I think I'd prefer to have a verifier check; I'll look into adding that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124697