[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