[PATCH] D94835: Add ehcont section support

[Arlo Siemsen via Phabricator via llvm-commits]

arlosi added inline comments.


================
Comment at: llvm/lib/CodeGen/EHContGuardCatchret.cpp:82
+      MCSymbol *CrSymbol = MF.getContext().getOrCreateSymbol(SymbolName);
+      MBBI->setPreInstrSymbol(MF, CrSymbol);
+      MF.addCatchretTarget(CrSymbol);
----------------
rnk wrote:
> This could be unreliable if some later pass reorders instructions or deletes the first instruction. I think it would be more reliable to take the address of the basic block. You can do this with `MBB->setHasAddressTaken` and `MachineModuleInfo::getAddrLabelSymbol`.
 `MachineModuleInfo::getAddrLabelSymbol` works on `BasicBlock`s rather than `MachineBasicBlocks`. So even if I `setHasAddressTaken` on the MBB, the associated BB might still not have it's address taken, which causes an assert failure in `getAddrLabelSymbolToEmit`.

I also tried using `MBB.setLabelMustBeEmitted()` and `MBB.getSymbol()`, but that produces a symbol name that doesn't end up in the symbol table.

We could add an additional field to `MachineBasicBlock` to store this special symbol, then emit it in the `AsmPrinter` in `emitBasicBlockStart`.

Do you have a preference, or another better solution?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94835