[PATCH] D153189: [MC] Make .pseudo_probe GC-able

[Fangrui Song via Phabricator via llvm-commits]

MaskRay marked 2 inline comments as done.
MaskRay added a comment.

Thanks for the quick review!



================
Comment at: llvm/lib/MC/MCObjectFileInfo.cpp:1179
+
+  return Ctx->getELFSection(PseudoProbeSection->getName(), ELF::SHT_PROGBITS,
+                            Flags, 0, GroupName, true, ElfSec.getUniqueID(),
----------------
hoy wrote:
> I think the type should inherit the type of `PseudoProbeSection`.
`getSection` is a MCELFSection method. We'd need `cast<MCELFSection>(PseudoProbeSection)->getType()` which is a bit too complex. Generally `SHT_PROGBITS` is the most common ELF section type and the type is checked by pseudo-probe-emit.ll. I hope that writing the type verbatim isn't too bad...


================
Comment at: llvm/lib/MC/MCObjectFileInfo.cpp:1180
+  return Ctx->getELFSection(PseudoProbeSection->getName(), ELF::SHT_PROGBITS,
+                            Flags, 0, GroupName, true, ElfSec.getUniqueID(),
+                            cast<MCSymbolELF>(TextSec.getBeginSymbol()));
----------------
hoy wrote:
> Should the pseudo probe section be a comdat only if the text section is a comdat? 
The `const MCSymbol *Group = ElfSec.getGroup()` condition ensures that the `.pseudo_probe` section is in a section group only when the text section is in a section group. When `GroupName` is empty, `getELFSection` does not use a section group.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153189