[PATCH] D95851: [MC][ELF] Support for zero flag section groups
Petr Hosek via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Sat Feb 13 22:23:41 PST 2021
phosek added inline comments.
================
Comment at: llvm/include/llvm/MC/MCContext.h:490
unsigned Flags, unsigned EntrySize,
- const Twine &Group) {
- return getELFSection(Section, Type, Flags, EntrySize, Group,
+ const Twine &Group, bool IsComdat = true) {
+ return getELFSection(Section, Type, Flags, EntrySize, Group, IsComdat,
----------------
MaskRay wrote:
> `Group == ""` means a non-SHF_GROUP section. `IsComdat = true` can be misleading for such use cases.
>
> We should pick a variable which defaults to false here.
>
> NoDeduplicates is a PE-COFF specific name, and we should stick with ELF naming here. So a concise name indicating zero section group flags should be used.
I used `IsComdat = true` because I was worried that updating every existing call site is going to be too much effort, but turned it wasn't so bad so I removed the default argument.
I think that having `/*IsComdat=*/true` or `/*IsComdat=*/false` explicitly in the code is actually nice because it's clear which type of group we're requesting.
Let me know what you think.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D95851/new/
https://reviews.llvm.org/D95851
More information about the llvm-commits
mailing list