[PATCH] D133456: [V2] MC: make section classification a bit more thorough
Fangrui Song via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Thu Sep 8 21:51:08 PDT 2022
MaskRay added inline comments.
================
Comment at: llvm/lib/MC/MCContext.cpp:567
Kind = SectionKind::getText();
- else
+ else if (~Flags & ELF::SHF_WRITE)
Kind = SectionKind::getReadOnly();
----------------
I think `!(... & ...)` is more common
================
Comment at: llvm/lib/MC/MCContext.cpp:576
+ .StartsWith(".bss.", SectionKind::getBSS())
+ .StartsWith(".gnu.linkonce.b.", SectionKind::getBSS())
+ .StartsWith(".llvm.linkonce.b.", SectionKind::getBSS())
----------------
Drop all `gnu.linkonce` and `llvm.linkonce`. linkonce is used before COMDAT era (<2000) and llvm never generates it.
================
Comment at: llvm/lib/MC/MCContext.cpp:580
+ .Case(".data1", SectionKind::getData())
+ .Case(".data.rel.ro", SectionKind::getReadOnlyWithRel())
+ .Case(".rodata", SectionKind::getReadOnly())
----------------
`.data.rel.ro.*` is possible.
================
Comment at: llvm/lib/MC/MCContext.cpp:583
+ .Case(".rodata1", SectionKind::getReadOnly())
+ .Case(".tbss", SectionKind::getThreadBSS())
+ .StartsWith(".tbss.", SectionKind::getThreadData())
----------------
Instead of a StringSwitch, consider
```
if (Name.consumes_front(".tbss") && (Name.empty() || Name[0] == '.'))
...
```
so that the many prefixes don't have to be repeated for Case/StartsWith.
================
Comment at: llvm/lib/MC/MCContext.cpp:592
+ .StartsWith(".debug_", SectionKind::getMetadata())
+ .Default(SectionKind::getText());
----------------
Is default `SectionKind::getText()` instead of `SectionKind::getReadOnly()` desired? I expect that text is covered by a previous `if`: `Flags & ELF::SHF_EXECINSTR`
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D133456/new/
https://reviews.llvm.org/D133456
More information about the llvm-commits
mailing list