[llvm] r239428 - Make MCSymbol::Name be a union of uint64_t and a pointer.

Pete Cooper peter_cooper at apple.com
Tue Jun 9 12:56:05 PDT 2015


Author: pete
Date: Tue Jun  9 14:56:05 2015
New Revision: 239428

URL: http://llvm.org/viewvc/llvm-project?rev=239428&view=rev
Log:
Make MCSymbol::Name be a union of uint64_t and a pointer.

This should hopefully fix the 32-bit bots which were allocating space for a pointer
but needed to be aligned to 64-bits.

Now we allocate enough space for a uint64_t and a pointer and cast to the appropriate storage

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=239428&r1=239427&r2=239428&view=diff
==============================================================================
--- llvm/trunk/include/llvm/MC/MCSymbol.h (original)
+++ llvm/trunk/include/llvm/MC/MCSymbol.h Tue Jun  9 14:56:05 2015
@@ -120,20 +120,29 @@ protected: // MCContext creates and uniq
   friend class MCExpr;
   friend class MCContext;
 
-  typedef const StringMapEntry<bool> NameEntryTy;
-  MCSymbol(SymbolKind Kind, NameEntryTy *Name, bool isTemporary)
+  /// \brief The name for a symbol.
+  /// MCSymbol contains a uint64_t so is probably aligned to 8.  On a 32-bit
+  /// system, the name is a pointer so isn't going to satisfy the 8 byte
+  /// alignment of uint64_t.  Account for that here.
+  typedef union {
+    const StringMapEntry<bool> *NameEntry;
+    uint64_t AlignmentPadding;
+  } NameEntryStorageTy;
+
+  MCSymbol(SymbolKind Kind, const StringMapEntry<bool> *Name, bool isTemporary)
       : Value(nullptr), IsTemporary(isTemporary),
         IsRedefinable(false), IsUsed(false), IsRegistered(false),
         IsExternal(false), IsPrivateExtern(false), HasName(!!Name),
         Kind(Kind) {
     Offset = 0;
     if (Name)
-      getNameEntryPtr() = Name;
+      getNameEntryPtr().NameEntry = Name;
   }
 
   // Provide custom new/delete as we will only allocate space for a name
   // if we need one.
-  void *operator new(size_t s, NameEntryTy *Name, MCContext &Ctx);
+  void *operator new(size_t s, const StringMapEntry<bool> *Name,
+                     MCContext &Ctx);
 
 private:
 
@@ -160,14 +169,14 @@ private:
   }
 
   /// \brief Get a reference to the name field.  Requires that we have a name
-  NameEntryTy *&getNameEntryPtr() {
+  NameEntryStorageTy &getNameEntryPtr() {
     assert(HasName && "Name is required");
-    NameEntryTy **Name = reinterpret_cast<NameEntryTy **>(this);
+    NameEntryStorageTy *Name = reinterpret_cast<NameEntryStorageTy *>(this);
     return *(Name - 1);
   }
-  NameEntryTy *const &getNameEntryPtr() const {
+  const NameEntryStorageTy &getNameEntryPtr() const {
     assert(HasName && "Name is required");
-    NameEntryTy *const *Name = reinterpret_cast<NameEntryTy *const *>(this);
+    const auto *Name = reinterpret_cast<const NameEntryStorageTy *>(this);
     return *(Name - 1);
   }
 
@@ -177,7 +186,7 @@ public:
     if (!HasName)
       return StringRef();
 
-    return getNameEntryPtr()->first();
+    return getNameEntryPtr().NameEntry->first();
   }
 
   bool isRegistered() const { return IsRegistered; }

Modified: llvm/trunk/lib/MC/MCSymbol.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/MC/MCSymbol.cpp?rev=239428&r1=239427&r2=239428&view=diff
==============================================================================
--- llvm/trunk/lib/MC/MCSymbol.cpp (original)
+++ llvm/trunk/lib/MC/MCSymbol.cpp Tue Jun  9 14:56:05 2015
@@ -19,17 +19,20 @@ using namespace llvm;
 // Sentinel value for the absolute pseudo section.
 MCSection *MCSymbol::AbsolutePseudoSection = reinterpret_cast<MCSection *>(1);
 
-void *MCSymbol::operator new(size_t s, NameEntryTy *Name, MCContext &Ctx) {
-  size_t Size = s + (Name ? sizeof(Name) : 0);
+void *MCSymbol::operator new(size_t s, const StringMapEntry<bool> *Name,
+                             MCContext &Ctx) {
+  // We may need more space for a Name to account for alignment.  So allocate
+  // space for the storage type and not the name pointer.
+  size_t Size = s + (Name ? sizeof(NameEntryStorageTy) : 0);
 
   // For safety, ensure that the alignment of a pointer is enough for an
   // MCSymbol.  This also ensures we don't need padding between the name and
   // symbol.
-  assert(alignOf<MCSymbol>() <= alignOf<NameEntryTy *>() &&
+  assert(alignOf<MCSymbol>() <= alignOf<NameEntryStorageTy>() &&
          "Bad alignment of MCSymbol");
-  void *Storage = Ctx.allocate(Size, alignOf<NameEntryTy *>());
-  NameEntryTy **Start = static_cast<NameEntryTy**>(Storage);
-  NameEntryTy **End = Start + (Name ? 1 : 0);
+  void *Storage = Ctx.allocate(Size, alignOf<NameEntryStorageTy>());
+  NameEntryStorageTy *Start = static_cast<NameEntryStorageTy*>(Storage);
+  NameEntryStorageTy *End = Start + (Name ? 1 : 0);
   return End;
 }
 





More information about the llvm-commits mailing list