[Alignment][NFC] Use Align in MCSymbol::setCommon

[gchatelet]

• rebase

[courbet]

llvm/include/llvm/MC/MCSymbol.h

345

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);

[gchatelet]

llvm/include/llvm/MC/MCSymbol.h

345

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.

llvm/include/llvm/MC/MCSymbol.h

uint64_t getCommonSize() const {
     return CommonSize;
   }
 
   /// Mark this symbol as being 'common'.
   ///
   /// \param Size - The size of the symbol.
   /// \param Align - The alignment of the symbol.
   /// \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);
     CommonSize = Size;
     SymbolContents = Target ? SymContentsTargetCommon : SymContentsCommon;
 
-    assert((!Align || isPowerOf2_32(Align)) &&
-           "Alignment must be a power of 2");
-    unsigned Log2Align = Log2_32(Align) + 1;
+    unsigned Log2Align = encode(Align);
     assert(Log2Align < (1U << NumCommonAlignmentBits) &&
            "Out of range alignment");
     CommonAlignLog2 = Log2Align;
   }
 
   ///  Return the alignment of a 'common' symbol.
   unsigned getCommonAlignment() const {
     assert(isCommon() && "Not a 'common' symbol!");
     return CommonAlignLog2 ? (1U << (CommonAlignLog2 - 1)) : 0;
   }
 
   /// Declare this symbol as being 'common'.
   ///
   /// \param Size - The size of the symbol.
   /// \param Align - The alignment of the symbol.
   /// \param Target - Is the symbol a target-specific common-like symbol.
   /// \return True if symbol was already declared as a different type
   bool declareCommon(uint64_t Size, unsigned Align, bool Target = false) {
     assert(isCommon() || getOffset() == 0);
     if(isCommon()) {
       if (CommonSize != Size || getCommonAlignment() != Align ||
           isTargetCommon() != Target)
         return true;
     } else
-      setCommon(Size, Align, Target);
+      setCommon(Size, llvm::Align(Align), Target);
     return false;
   }
 
   /// Is this a 'common' symbol.
   bool isCommon() const {
     return SymbolContents == SymContentsCommon ||
            SymbolContents == SymContentsTargetCommon;
   }

llvm/lib/MC/MCMachOStreamer.cpp

 
 void MCMachOStreamer::emitCommonSymbol(MCSymbol *Symbol, uint64_t Size,
                                        unsigned ByteAlignment) {
   // FIXME: Darwin 'as' does appear to allow redef of a .comm by itself.
   assert(Symbol->isUndefined() && "Cannot define a symbol twice!");
 
   getAssembler().registerSymbol(*Symbol);
   Symbol->setExternal(true);
-  Symbol->setCommon(Size, ByteAlignment);
+  Symbol->setCommon(Size, Align(ByteAlignment));
 }
 
 void MCMachOStreamer::emitLocalCommonSymbol(MCSymbol *Symbol, uint64_t Size,
                                             unsigned ByteAlignment) {
   // '.lcomm' is equivalent to '.zerofill'.
   return emitZerofill(getContext().getObjectFileInfo()->getDataBSSSection(),
                       Symbol, Size, ByteAlignment);
 }

llvm/lib/MC/MCWinCOFFStreamer.cpp

if (ByteAlignment > 32)
       report_fatal_error("alignment is limited to 32-bytes");
 
     // Round size up to alignment so that we will honor the alignment request.
     Size = std::max(Size, static_cast<uint64_t>(ByteAlignment));
   }
 
   getAssembler().registerSymbol(*Symbol);
   Symbol->setExternal(true);
-  Symbol->setCommon(Size, ByteAlignment);
+  Symbol->setCommon(Size, Align(ByteAlignment));
 
   if (!T.isWindowsMSVCEnvironment() && ByteAlignment > 1) {
     SmallString<128> Directive;
     raw_svector_ostream OS(Directive);
     const MCObjectFileInfo *MFI = getContext().getObjectFileInfo();
 
     OS << " -aligncomm:\"" << Symbol->getName() << "\","
        << Log2_32_Ceil(ByteAlignment);

llvm/lib/MC/MCXCOFFStreamer.cpp

getAssembler().getWriter().addExceptionEntry(Symbol, Trap, Lang, Reason,
                                                FunctionSize, hasDebug);
 }
 
 void MCXCOFFStreamer::emitCommonSymbol(MCSymbol *Symbol, uint64_t Size,
                                        unsigned ByteAlignment) {
   getAssembler().registerSymbol(*Symbol);
   Symbol->setExternal(cast<MCSymbolXCOFF>(Symbol)->getStorageClass() !=
                       XCOFF::C_HIDEXT);
-  Symbol->setCommon(Size, ByteAlignment);
+  Symbol->setCommon(Size, Align(ByteAlignment));
 
   // Default csect align is 4, but common symbols have explicit alignment values
   // and we should honor it.
   cast<MCSymbolXCOFF>(Symbol)->getRepresentedCsect()->setAlignment(
       Align(ByteAlignment));
 
   // Emit the alignment and storage for the variable to the section.
   emitValueToAlignment(Align(ByteAlignment));