Revert "Pack the MCSymbolELF bit fields into MCSymbol's Flags."

This reverts commit r239006.

I am debugging the powerpc failures.

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@239010 91177308-0d34-0410-b5e6-96231b3b80d8

include/llvm/MC/MCSymbolELF.h

@@ -17,9 +17,15 @@ class MCSymbolELF : public MCSymbol {
   /// symbol has no size this field will be NULL.
   const MCExpr *SymbolSize = nullptr;
 
+  mutable unsigned BindingSet : 1;
+  mutable unsigned UsedInReloc : 1;
+  mutable unsigned WeakrefUsedInReloc : 1;
+  mutable unsigned IsSignature : 1;
+
 public:
   MCSymbolELF(const StringMapEntry<bool> *Name, bool isTemporary)
-      : MCSymbol(true, Name, isTemporary) {}
+      : MCSymbol(true, Name, isTemporary), BindingSet(false),
+        UsedInReloc(false), WeakrefUsedInReloc(false), IsSignature(false) {}
   void setSize(const MCExpr *SS) { SymbolSize = SS; }
 
   const MCExpr *getSize() const { return SymbolSize; }
@@ -36,7 +42,7 @@ public:
   void setBinding(unsigned Binding) const;
   unsigned getBinding() const;
 
-  bool isBindingSet() const;
+  bool isBindingSet() const { return BindingSet; }
 
   void setUsedInReloc() const;
   bool isUsedInReloc() const;
@@ -48,9 +54,6 @@ public:
   bool isSignature() const;
 
   static bool classof(const MCSymbol *S) { return S->isELF(); }
-
-private:
-  void setIsBindingSet() const;
 };
 }
 

lib/MC/ELFObjectWriter.cpp

@@ -463,7 +463,8 @@ void ELFObjectWriter::writeSymbol(SymbolTableWriter &Writer,
   // Other and Visibility share the same byte with Visibility using the lower
   // 2 bits
   uint8_t Visibility = Symbol.getVisibility();
-  uint8_t Other = Symbol.getOther() | Visibility;
+  uint8_t Other = Symbol.getOther() << 2;
+  Other |= Visibility;
 
   uint64_t Value = SymbolValue(*MSD.Symbol, Layout);
   uint64_t Size = 0;

lib/MC/MCSymbolELF.cpp

@@ -16,71 +16,27 @@ namespace llvm {
 
 namespace {
 enum {
-  // Shift value for STT_* flags. 7 possible values. 3 bits.
+  ELF_STT_Shift = 0, // Shift value for STT_* flags
-  ELF_STT_Shift = 0,
+  ELF_STB_Shift = 4, // Shift value for STB_* flags
-
+  ELF_STV_Shift = 8, // Shift value for STV_* flags
-  // Shift value for STB_* flags. 4 possible values, 2 bits.
+  ELF_STO_Shift = 10 // Shift value for STO_* flags
-  ELF_STB_Shift = 3,
-
-  // Shift value for STV_* flags. 4 possible values, 2 bits.
-  ELF_STV_Shift = 5,
-
-  // Shift value for STO_* flags. 3 bits. All the values are between 0x20 and
-  // 0xe0, so we shift right by 5 before storing.
-  ELF_STO_Shift = 7,
-
-  // One bit.
-  ELF_IsSignature_Shift = 10,
-
-  // One bit.
-  ELF_WeakrefUsedInReloc_Shift = 11,
-
-  // One bit.
-  ELF_UsedInReloc_Shift = 12,
-
-  // One bit.
-  ELF_BindingSet_Shift = 13
 };
 }
 
 void MCSymbolELF::setBinding(unsigned Binding) const {
-  setIsBindingSet();
+  BindingSet = true;
-  unsigned Val;
+  assert(Binding == ELF::STB_LOCAL || Binding == ELF::STB_GLOBAL ||
-  switch (Binding) {
+         Binding == ELF::STB_WEAK || Binding == ELF::STB_GNU_UNIQUE);
-  default:
+  uint32_t OtherFlags = getFlags() & ~(0xf << ELF_STB_Shift);
-    llvm_unreachable("Unsupported Binding");
+  setFlags(OtherFlags | (Binding << ELF_STB_Shift));
-  case ELF::STB_LOCAL:
-    Val = 0;
-    break;
-  case ELF::STB_GLOBAL:
-    Val = 1;
-    break;
-  case ELF::STB_WEAK:
-    Val = 2;
-    break;
-  case ELF::STB_GNU_UNIQUE:
-    Val = 3;
-    break;
-  }
-  uint32_t OtherFlags = getFlags() & ~(0x3 << ELF_STB_Shift);
-  setFlags(OtherFlags | (Val << ELF_STB_Shift));
 }
 
 unsigned MCSymbolELF::getBinding() const {
   if (isBindingSet()) {
-    uint32_t Val = (getFlags() & (0x3 << ELF_STB_Shift)) >> ELF_STB_Shift;
+    uint32_t Binding = (getFlags() & (0xf << ELF_STB_Shift)) >> ELF_STB_Shift;
-    switch (Val) {
+    assert(Binding == ELF::STB_LOCAL || Binding == ELF::STB_GLOBAL ||
-    default:
+           Binding == ELF::STB_WEAK || Binding == ELF::STB_GNU_UNIQUE);
-      llvm_unreachable("Invalid value");
+    return Binding;
-    case 0:
-      return ELF::STB_LOCAL;
-    case 1:
-      return ELF::STB_GLOBAL;
-    case 2:
-      return ELF::STB_WEAK;
-    case 3:
-      return ELF::STB_GNU_UNIQUE;
-    }
   }
 
   if (isDefined())
@@ -95,58 +51,26 @@ unsigned MCSymbolELF::getBinding() const {
 }
 
 void MCSymbolELF::setType(unsigned Type) const {
-  unsigned Val;
+  assert(Type == ELF::STT_NOTYPE || Type == ELF::STT_OBJECT ||
-  switch (Type) {
+         Type == ELF::STT_FUNC || Type == ELF::STT_SECTION ||
-  default:
+         Type == ELF::STT_COMMON || Type == ELF::STT_TLS ||
-    llvm_unreachable("Unsupported Binding");
+         Type == ELF::STT_GNU_IFUNC);
-  case ELF::STT_NOTYPE:
+
-    Val = 0;
+  uint32_t OtherFlags = getFlags() & ~(0xf << ELF_STT_Shift);
-    break;
+  setFlags(OtherFlags | (Type << ELF_STT_Shift));
-  case ELF::STT_OBJECT:
-    Val = 1;
-    break;
-  case ELF::STT_FUNC:
-    Val = 2;
-    break;
-  case ELF::STT_SECTION:
-    Val = 3;
-    break;
-  case ELF::STT_COMMON:
-    Val = 4;
-    break;
-  case ELF::STT_TLS:
-    Val = 5;
-    break;
-  case ELF::STT_GNU_IFUNC:
-    Val = 6;
-    break;
-  }
-  uint32_t OtherFlags = getFlags() & ~(0x7 << ELF_STT_Shift);
-  setFlags(OtherFlags | (Val << ELF_STT_Shift));
 }
 
 unsigned MCSymbolELF::getType() const {
-  uint32_t Val = (getFlags() & (0x7 << ELF_STT_Shift)) >> ELF_STT_Shift;
+  uint32_t Type = (getFlags() & (0xf << ELF_STT_Shift)) >> ELF_STT_Shift;
-  switch (Val) {
+  assert(Type == ELF::STT_NOTYPE || Type == ELF::STT_OBJECT ||
-  default:
+         Type == ELF::STT_FUNC || Type == ELF::STT_SECTION ||
-    llvm_unreachable("Invalid value");
+         Type == ELF::STT_COMMON || Type == ELF::STT_TLS ||
-  case 0:
+         Type == ELF::STT_GNU_IFUNC);
-    return ELF::STT_NOTYPE;
+  return Type;
-  case 1:
-    return ELF::STT_OBJECT;
-  case 2:
-    return ELF::STT_FUNC;
-  case 3:
-    return ELF::STT_SECTION;
-  case 4:
-    return ELF::STT_COMMON;
-  case 5:
-    return ELF::STT_TLS;
-  case 6:
-    return ELF::STT_GNU_IFUNC;
-  }
 }
 
+// Visibility is stored in the first two bits of st_other
+// st_other values are stored in the second byte of get/setFlags
 void MCSymbolELF::setVisibility(unsigned Visibility) {
   assert(Visibility == ELF::STV_DEFAULT || Visibility == ELF::STV_INTERNAL ||
          Visibility == ELF::STV_HIDDEN || Visibility == ELF::STV_PROTECTED);
@@ -162,52 +86,31 @@ unsigned MCSymbolELF::getVisibility() const {
   return Visibility;
 }
 
+// Other is stored in the last six bits of st_other
+// st_other values are stored in the second byte of get/setFlags
 void MCSymbolELF::setOther(unsigned Other) {
-  assert((Other & 0x1f) == 0);
+  uint32_t OtherFlags = getFlags() & ~(0x3f << ELF_STO_Shift);
-  Other >>= 5;
-  assert(Other <= 0x7);
-  uint32_t OtherFlags = getFlags() & ~(0x7 << ELF_STO_Shift);
   setFlags(OtherFlags | (Other << ELF_STO_Shift));
 }
 
 unsigned MCSymbolELF::getOther() const {
   unsigned Other = (getFlags() & (0x3f << ELF_STO_Shift)) >> ELF_STO_Shift;
-  return Other << 5;
+  return Other;
 }
 
 void MCSymbolELF::setUsedInReloc() const {
-  uint32_t OtherFlags = getFlags() & ~(0x1 << ELF_UsedInReloc_Shift);
+  UsedInReloc = true;
-  setFlags(OtherFlags | (1 << ELF_UsedInReloc_Shift));
 }
 
 bool MCSymbolELF::isUsedInReloc() const {
-  return getFlags() & (0x1 << ELF_UsedInReloc_Shift);
+  return UsedInReloc;
 }
 
-void MCSymbolELF::setIsWeakrefUsedInReloc() const {
+void MCSymbolELF::setIsWeakrefUsedInReloc() const { WeakrefUsedInReloc = true; }
-  uint32_t OtherFlags = getFlags() & ~(0x1 << ELF_WeakrefUsedInReloc_Shift);
-  setFlags(OtherFlags | (1 << ELF_WeakrefUsedInReloc_Shift));
-}
 
-bool MCSymbolELF::isWeakrefUsedInReloc() const {
+bool MCSymbolELF::isWeakrefUsedInReloc() const { return WeakrefUsedInReloc; }
-  return getFlags() & (0x1 << ELF_WeakrefUsedInReloc_Shift);
-}
 
-void MCSymbolELF::setIsSignature() const {
+void MCSymbolELF::setIsSignature() const { IsSignature = true; }
-  uint32_t OtherFlags = getFlags() & ~(0x1 << ELF_IsSignature_Shift);
-  setFlags(OtherFlags | (1 << ELF_IsSignature_Shift));
-}
 
-bool MCSymbolELF::isSignature() const {
+bool MCSymbolELF::isSignature() const { return IsSignature; }
-  return getFlags() & (0x1 << ELF_IsSignature_Shift);
-}
-
-void MCSymbolELF::setIsBindingSet() const {
-  uint32_t OtherFlags = getFlags() & ~(0x1 << ELF_BindingSet_Shift);
-  setFlags(OtherFlags | (1 << ELF_BindingSet_Shift));
-}
-
-bool MCSymbolELF::isBindingSet() const {
-  return getFlags() & (0x1 << ELF_BindingSet_Shift);
-}
 }

lib/Target/Mips/MCTargetDesc/MipsELFObjectWriter.cpp

@@ -384,7 +384,7 @@ bool MipsELFObjectWriter::needsRelocateWithSymbol(const MCSymbol &Sym,
     return true;
 
   case ELF::R_MIPS_32:
-    if (cast<MCSymbolELF>(Sym).getOther() & ELF::STO_MIPS_MICROMIPS)
+    if (cast<MCSymbolELF>(Sym).getOther() & (ELF::STO_MIPS_MICROMIPS >> 2))
       return true;
     // falltrough
   case ELF::R_MIPS_26:

lib/Target/Mips/MCTargetDesc/MipsELFStreamer.cpp

@@ -44,7 +44,10 @@ void MipsELFStreamer::createPendingLabelRelocs() {
     for (auto *L : Labels) {
       auto *Label = cast<MCSymbolELF>(L);
       getAssembler().registerSymbol(*Label);
-      Label->setOther(ELF::STO_MIPS_MICROMIPS);
+      // The "other" values are stored in the last 6 bits of the second byte.
+      // The traditional defines for STO values assume the full byte and thus
+      // the shift to pack it.
+      Label->setOther(ELF::STO_MIPS_MICROMIPS >> 2);
     }
   }
 

lib/Target/Mips/MCTargetDesc/MipsTargetStreamer.cpp

@@ -462,7 +462,10 @@ void MipsTargetELFStreamer::emitLabel(MCSymbol *S) {
   if (Type != ELF::STT_FUNC)
     return;
 
-  Symbol->setOther(ELF::STO_MIPS_MICROMIPS);
+  // The "other" values are stored in the last 6 bits of the second byte
+  // The traditional defines for STO values assume the full byte and thus
+  // the shift to pack it.
+  Symbol->setOther(ELF::STO_MIPS_MICROMIPS >> 2);
 }
 
 void MipsTargetELFStreamer::finish() {
@@ -524,10 +527,13 @@ void MipsTargetELFStreamer::emitAssignment(MCSymbol *S, const MCExpr *Value) {
   const auto &RhsSym = cast<MCSymbolELF>(
       static_cast<const MCSymbolRefExpr *>(Value)->getSymbol());
 
-  if (!(RhsSym.getOther() & ELF::STO_MIPS_MICROMIPS))
+  if (!(RhsSym.getOther() & (ELF::STO_MIPS_MICROMIPS >> 2)))
     return;
 
-  Symbol->setOther(ELF::STO_MIPS_MICROMIPS);
+  // The "other" values are stored in the last 6 bits of the second byte.
+  // The traditional defines for STO values assume the full byte and thus
+  // the shift to pack it.
+  Symbol->setOther(ELF::STO_MIPS_MICROMIPS >> 2);
 }
 
 MCELFStreamer &MipsTargetELFStreamer::getStreamer() {

lib/Target/PowerPC/MCTargetDesc/PPCMCTargetDesc.cpp

@@ -169,10 +169,13 @@ public:
     if (Res != ELF::decodePPC64LocalEntryOffset(Encoded))
       report_fatal_error(".localentry expression cannot be encoded.");
 
-    unsigned Other = S->getOther();
+    // The "other" values are stored in the last 6 bits of the second byte.
+    // The traditional defines for STO values assume the full byte and thus
+    // the shift to pack it.
+    unsigned Other = S->getOther() << 2;
     Other &= ~ELF::STO_PPC64_LOCAL_MASK;
     Other |= Encoded;
-    S->setOther(Other);
+    S->setOther(Other >> 2);
 
     // For GAS compatibility, unless we already saw a .abiversion directive,
     // set e_flags to indicate ELFv2 ABI.
@@ -188,10 +191,13 @@ public:
       return;
     const auto &RhsSym = cast<MCSymbolELF>(
         static_cast<const MCSymbolRefExpr *>(Value)->getSymbol());
-    unsigned Other = Symbol->getOther();
+    // The "other" values are stored in the last 6 bits of the second byte.
+    // The traditional defines for STO values assume the full byte and thus
+    // the shift to pack it.
+    unsigned Other = Symbol->getOther() << 2;
     Other &= ~ELF::STO_PPC64_LOCAL_MASK;
-    Other |= RhsSym.getOther() & ELF::STO_PPC64_LOCAL_MASK;
+    Other |= (RhsSym.getOther() << 2) & ELF::STO_PPC64_LOCAL_MASK;
-    Symbol->setOther(Other);
+    Symbol->setOther(Other >> 2);
   }
 };