[llvm] r240320 - Move MCSymbol Value in to the union of Offset and CommonSize.

[Pete Cooper]

Author: pete
Date: Mon Jun 22 14:57:33 2015
New Revision: 240320

URL: http://llvm.org/viewvc/llvm-project?rev=240320&view=rev
Log:
Move MCSymbol Value in to the union of Offset and CommonSize.

This is a reapplication of r239440 which was reverted in r239441.
There are no changes to this patch from then, but this had instead exposed
a bug in .thumb_set which was fixed in r240318.  Having fixed that bug, it
is now safe to re-apply this code.

Original commit message below:

It wasn't possible to have a variable Symbol with offset or 'isCommon' so
this just enables better packing of the MCSymbol class.

Reviewed by Rafael Espindola.

Modified:
    llvm/trunk/include/llvm/MC/MCSymbol.h
    llvm/trunk/lib/MC/MCSymbol.cpp

Modified: llvm/trunk/include/llvm/MC/MCSymbol.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/MC/MCSymbol.h?rev=240320&r1=240319&r2=240320&view=diff
==============================================================================
--- llvm/trunk/include/llvm/MC/MCSymbol.h (original)
+++ llvm/trunk/include/llvm/MC/MCSymbol.h Mon Jun 22 14:57:33 2015
@@ -46,6 +46,15 @@ protected:
     SymbolKindMachO,
   };
 
+  /// A symbol can contain an Offset, or Value, or be Common, but never more
+  /// than one of these.
+  enum Contents : uint8_t {
+    SymContentsUnset,
+    SymContentsOffset,
+    SymContentsVariable,
+    SymContentsCommon,
+  };
+
   // Special sentinal value for the absolute pseudo section.
   //
   // FIXME: Use a PointerInt wrapper for this?
@@ -64,9 +73,6 @@ protected:
   /// relative to, if any.
   mutable PointerUnion<MCSection *, MCFragment *> SectionOrFragment;
 
-  /// Value - If non-null, the value for a variable symbol.
-  const MCExpr *Value;
-
   /// IsTemporary - True if this is an assembler temporary label, which
   /// typically does not survive in the .o file's symbol table.  Usually
   /// "Lfoo" or ".foo".
@@ -98,6 +104,10 @@ protected:
   /// True if we have created a relocation that uses this symbol.
   mutable unsigned IsUsedInReloc : 1;
 
+  /// This is actually a Contents enumerator, but is unsigned to avoid sign
+  /// extension and achieve better bitpacking with MSVC.
+  unsigned SymbolContents : 2;
+
   /// Index field, for use by the object file implementation.
   mutable uint32_t Index = 0;
 
@@ -107,6 +117,9 @@ protected:
 
     /// The size of the symbol, if it is 'common'.
     uint64_t CommonSize;
+
+    /// If non-null, the value for a variable symbol.
+    const MCExpr *Value;
   };
 
   /// The alignment of the symbol, if it is 'common', or -1.
@@ -132,10 +145,10 @@ protected: // MCContext creates and uniq
   } NameEntryStorageTy;
 
   MCSymbol(SymbolKind Kind, const StringMapEntry<bool> *Name, bool isTemporary)
-      : Value(nullptr), IsTemporary(isTemporary), IsRedefinable(false),
+      : IsTemporary(isTemporary), IsRedefinable(false),
         IsUsed(false), IsRegistered(false), IsExternal(false),
         IsPrivateExtern(false), HasName(!!Name), Kind(Kind),
-        IsUsedInReloc(false) {
+        IsUsedInReloc(false), SymbolContents(SymContentsUnset) {
     Offset = 0;
     if (Name)
       getNameEntryPtr() = Name;
@@ -165,9 +178,9 @@ private:
       return F->getParent();
     assert(!SectionOrFragment.is<MCFragment *>() && "Section or null expected");
     MCSection *Section = SectionOrFragment.dyn_cast<MCSection *>();
-    if (Section || !Value)
+    if (Section || !isVariable())
       return Section;
-    return Section = Value->findAssociatedSection();
+    return Section = getVariableValue()->findAssociatedSection();
   }
 
   /// \brief Get a reference to the name field.  Requires that we have a name
@@ -212,7 +225,10 @@ public:
   /// \brief Prepare this symbol to be redefined.
   void redefineIfPossible() {
     if (IsRedefinable) {
-      Value = nullptr;
+      if (SymbolContents == SymContentsVariable) {
+        Value = nullptr;
+        SymbolContents = SymContentsUnset;
+      }
       SectionOrFragment = nullptr;
       IsRedefinable = false;
     }
@@ -266,7 +282,9 @@ public:
   /// @{
 
   /// isVariable - Check if this is a variable symbol.
-  bool isVariable() const { return Value != nullptr; }
+  bool isVariable() const {
+    return SymbolContents == SymContentsVariable;
+  }
 
   /// getVariableValue() - Get the value for variable symbols.
   const MCExpr *getVariableValue() const {
@@ -290,12 +308,17 @@ public:
   }
 
   uint64_t getOffset() const {
-    assert(!isCommon());
+    assert((SymbolContents == SymContentsUnset ||
+            SymbolContents == SymContentsOffset) &&
+           "Cannot get offset for a common/variable symbol");
     return Offset;
   }
   void setOffset(uint64_t Value) {
-    assert(!isCommon());
+    assert((SymbolContents == SymContentsUnset ||
+            SymbolContents == SymContentsOffset) &&
+           "Cannot set offset for a common/variable symbol");
     Offset = Value;
+    SymbolContents = SymContentsOffset;
   }
 
   /// Return the size of a 'common' symbol.
@@ -312,6 +335,7 @@ public:
     assert(getOffset() == 0);
     CommonSize = Size;
     CommonAlign = Align;
+    SymbolContents = SymContentsCommon;
   }
 
   ///  Return the alignment of a 'common' symbol.
@@ -336,7 +360,9 @@ public:
   }
 
   /// Is this a 'common' symbol.
-  bool isCommon() const { return CommonAlign != -1U; }
+  bool isCommon() const {
+    return SymbolContents == SymContentsCommon;
+  }
 
   MCFragment *getFragment() const {
     return SectionOrFragment.dyn_cast<MCFragment *>();

Modified: llvm/trunk/lib/MC/MCSymbol.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/MC/MCSymbol.cpp?rev=240320&r1=240319&r2=240320&view=diff
==============================================================================
--- llvm/trunk/lib/MC/MCSymbol.cpp (original)
+++ llvm/trunk/lib/MC/MCSymbol.cpp Mon Jun 22 14:57:33 2015
@@ -40,7 +40,11 @@ void *MCSymbol::operator new(size_t s, c
 void MCSymbol::setVariableValue(const MCExpr *Value) {
   assert(!IsUsed && "Cannot set a variable that has already been used.");
   assert(Value && "Invalid variable value!");
+  assert((SymbolContents == SymContentsUnset ||
+          SymbolContents == SymContentsVariable) &&
+         "Cannot give common/offset symbol a variable value");
   this->Value = Value;
+  SymbolContents = SymContentsVariable;
   SectionOrFragment = nullptr;
 }