[PATCH] D94835: Add ehcont section support

Reid Kleckner via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Jan 19 14:55:00 PST 2021


rnk added inline comments.


================
Comment at: llvm/lib/CodeGen/EHContGuardCatchret.cpp:80
+      raw_svector_ostream(SymbolName)
+          << "$ehgcr_" << MF.getName() << MBB.getNumber();
+      MCSymbol *CrSymbol = MF.getContext().getOrCreateSymbol(SymbolName);
----------------
If we do use this name, I would prefer to use as short a name as possible. C++ names are long enough that they significantly contribute to object file size. Let's use getFunctionNumber() here instead, so we get names like `$ehgcr_2_42`. You'll need a non-digit separator to ensure the names are unique (consider function 5 bb 55 and function 55 bb 5).

This wasn't as much of a concern for longjmp tables, since setjmp is less common and mostly appears in C code.


================
Comment at: llvm/lib/CodeGen/EHContGuardCatchret.cpp:82
+      MCSymbol *CrSymbol = MF.getContext().getOrCreateSymbol(SymbolName);
+      MBBI->setPreInstrSymbol(MF, CrSymbol);
+      MF.addCatchretTarget(CrSymbol);
----------------
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`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94835



More information about the llvm-commits mailing list