[PATCH] D138705: [Alignment][NFC] Use Align in MCSymbol::setCommon
Guillaume Chatelet via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Mon Nov 28 00:53:10 PST 2022
gchatelet planned changes to this revision.
gchatelet added inline comments.
================
Comment at: llvm/include/llvm/MC/MCSymbol.h:345
/// \param Target - Is the symbol a target-specific common-like symbol.
- void setCommon(uint64_t Size, unsigned Align, bool Target = false) {
+ void setCommon(uint64_t Size, Align Align, bool Target = false) {
assert(getOffset() == 0);
----------------
courbet wrote:
> I can't convince myself that all callers pass a non-zero value for `Align`. In particular, I found that `AsmPrinter::emitGlobalVariable` does:
>
> `OutStreamer->emitCommonSymbol(GVSym, Size, SupportsAlignment ? Alignment.value() : 0);`
Yes this has been added by mistake many years ago.
See https://reviews.llvm.org/D138721#3951130.
I'm preparing a patch to revert to the old behavior where 0 is not a valid input anymore. Once this is accepted this will be NFC.
Let's put the patch on hold until then.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D138705/new/
https://reviews.llvm.org/D138705
More information about the llvm-commits
mailing list