[PATCH] D139826: [Alignment][NFC] MCSymbol::getCommonAlignment returns MaybeAlign, improve documentation.

[Guillaume Chatelet via Phabricator via llvm-commits]

gchatelet created this revision.
gchatelet added a reviewer: courbet.
Herald added a subscriber: hiraditya.
Herald added a project: All.
gchatelet requested review of this revision.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

This one goes hand in hand with D139819 <https://reviews.llvm.org/D139819>


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D139826

Files:
  llvm/include/llvm/MC/MCSymbol.h
  llvm/include/llvm/MC/MCSymbolMachO.h
  llvm/lib/MC/ELFObjectWriter.cpp


Index: llvm/lib/MC/ELFObjectWriter.cpp
===================================================================
--- llvm/lib/MC/ELFObjectWriter.cpp
+++ llvm/lib/MC/ELFObjectWriter.cpp
@@ -454,7 +454,7 @@
 uint64_t ELFWriter::SymbolValue(const MCSymbol &Sym,
                                 const MCAsmLayout &Layout) {
   if (Sym.isCommon())
-    return Sym.getCommonAlignment();
+    return Sym.getCommonAlignment()->value();
 
   uint64_t Res;
   if (!Layout.getSymbolOffset(Sym, Res))
Index: llvm/include/llvm/MC/MCSymbolMachO.h
===================================================================
--- llvm/include/llvm/MC/MCSymbolMachO.h
+++ llvm/include/llvm/MC/MCSymbolMachO.h
@@ -115,12 +115,13 @@
 
     // Common alignment is packed into the 'desc' bits.
     if (isCommon()) {
-      if (unsigned Align = getCommonAlignment()) {
-        unsigned Log2Size = Log2_32(Align);
-        assert((1U << Log2Size) == Align && "Invalid 'common' alignment!");
+      if (MaybeAlign MaybeAlignment = getCommonAlignment()) {
+        Align Alignment = *MaybeAlignment;
+        unsigned Log2Size = Log2(Alignment);
         if (Log2Size > 15)
           report_fatal_error("invalid 'common' alignment '" +
-                             Twine(Align) + "' for '" + getName() + "'",
+                                 Twine(Alignment.value()) + "' for '" +
+                                 getName() + "'",
                              false);
         Flags = (Flags & SF_CommonAlignmentMask) |
                 (Log2Size << SF_CommonAlignmentShift);
Index: llvm/include/llvm/MC/MCSymbol.h
===================================================================
--- llvm/include/llvm/MC/MCSymbol.h
+++ llvm/include/llvm/MC/MCSymbol.h
@@ -113,11 +113,16 @@
   /// extension and achieve better bitpacking with MSVC.
   unsigned SymbolContents : 3;
 
-  /// The alignment of the symbol, if it is 'common', or -1.
+  /// The alignment of the symbol if it is 'common'.
   ///
-  /// The alignment is stored as log2(align) + 1.  This allows all values from
-  /// 0 to 2^31 to be stored which is every power of 2 representable by an
-  /// unsigned.
+  /// Internally, this is stored as log2(align) + 1.
+  /// We reserve 5 bits to encode this value which allows the following values
+  /// 0b00000 -> unset
+  /// 0b00001 -> 1ULL <<  0 = 1
+  /// 0b00010 -> 1ULL <<  1 = 2
+  /// 0b00011 -> 1ULL <<  2 = 4
+  /// ...
+  /// 0b11111 -> 1ULL << 30 = 1 GiB
   enum : unsigned { NumCommonAlignmentBits = 5 };
   unsigned CommonAlignLog2 : NumCommonAlignmentBits;
 
@@ -356,25 +361,25 @@
   }
 
   ///  Return the alignment of a 'common' symbol.
-  unsigned getCommonAlignment() const {
+  MaybeAlign getCommonAlignment() const {
     assert(isCommon() && "Not a 'common' symbol!");
-    return CommonAlignLog2 ? (1U << (CommonAlignLog2 - 1)) : 0;
+    return decodeMaybeAlign(CommonAlignLog2);
   }
 
   /// Declare this symbol as being 'common'.
   ///
   /// \param Size - The size of the symbol.
-  /// \param Align - The alignment of the symbol.
+  /// \param Alignment - 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) {
+  bool declareCommon(uint64_t Size, unsigned Alignment, bool Target = false) {
     assert(isCommon() || getOffset() == 0);
     if(isCommon()) {
-      if (CommonSize != Size || getCommonAlignment() != Align ||
+      if (CommonSize != Size || getCommonAlignment() != Align(Alignment) ||
           isTargetCommon() != Target)
         return true;
     } else
-      setCommon(Size, Align, Target);
+      setCommon(Size, Alignment, Target);
     return false;
   }
 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D139826.482050.patch
Type: text/x-patch
Size: 3778 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20221212/0ee20a85/attachment.bin>