Centralize the handling of the thumb bit.

This patch centralizes the handling of the thumb bit around
MCStreamer::isThumbFunc and makes isThumbFunc handle aliases.

This fixes a corner case, but the main advantage is having just one
way to check if a MCSymbol is thumb or not. This should still be
refactored to be ARM only, but at least now it is just one predicate
that has to be refactored instead of 3 (isThumbFunc,
ELF_Other_ThumbFunc, and SF_ThumbFunc).

llvm-svn: 207522

include/llvm/MC/MCAssembler.h

@@ -902,7 +902,7 @@ private:
   // here. Maybe when the relocation stuff moves to target specific,
   // this can go with it? The streamer would need some target specific
   // refactoring too.
-  SmallPtrSet<const MCSymbol*, 64> ThumbFuncs;
+  mutable SmallPtrSet<const MCSymbol*, 64> ThumbFuncs;
 
   /// \brief The bundle alignment size currently set in the assembler.
   ///
@@ -995,9 +995,7 @@ public:
                         const MCAsmLayout &Layout) const;
 
   /// Check whether a given symbol has been flagged with .thumb_func.
-  bool isThumbFunc(const MCSymbol *Func) const {
-    return ThumbFuncs.count(Func);
-  }
+  bool isThumbFunc(const MCSymbol *Func) const;
 
   /// Flag a function symbol as the target of a .thumb_func directive.
   void setIsThumbFunc(const MCSymbol *Func) { ThumbFuncs.insert(Func); }

include/llvm/MC/MCELFSymbolFlags.h

@@ -24,9 +24,7 @@ namespace llvm {
     ELF_STT_Shift   = 0,  // Shift value for STT_* flags.
     ELF_STB_Shift   = 4,  // Shift value for STB_* flags.
     ELF_STV_Shift   = 8,  // Shift value for STV_* flags.
-    ELF_STO_Shift   = 10, // Shift value for STO_* flags.
-    ELF_Other_Shift = 16  // Shift value for llvm local flags,
-                          // not part of the final object file
+    ELF_STO_Shift   = 10  // Shift value for STO_* flags.
   };
 
   enum ELFSymbolFlags {
@@ -49,9 +47,7 @@ namespace llvm {
       ELF_STV_Default   = (ELF::STV_DEFAULT   << ELF_STV_Shift),
       ELF_STV_Internal  = (ELF::STV_INTERNAL  << ELF_STV_Shift),
       ELF_STV_Hidden    = (ELF::STV_HIDDEN    << ELF_STV_Shift),
-      ELF_STV_Protected = (ELF::STV_PROTECTED << ELF_STV_Shift),
-
-      ELF_Other_ThumbFunc = (1                << ELF_Other_Shift)
+      ELF_STV_Protected = (ELF::STV_PROTECTED << ELF_STV_Shift)
   };
 
 } // end namespace llvm

lib/MC/ELFObjectWriter.cpp

@@ -215,7 +215,8 @@ class ELFObjectWriter : public MCObjectWriter {
                           const MCAsmLayout &Layout,
                           SectionIndexMapTy &SectionIndexMap);
 
-    bool shouldRelocateWithSymbol(const MCSymbolRefExpr *RefA,
+    bool shouldRelocateWithSymbol(const MCAssembler &Asm,
+                                  const MCSymbolRefExpr *RefA,
                                   const MCSymbolData *SD, uint64_t C,
                                   unsigned Type) const;
 
@@ -486,6 +487,7 @@ void ELFObjectWriter::WriteHeader(const MCAssembler &Asm,
 
 uint64_t ELFObjectWriter::SymbolValue(MCSymbolData &OrigData,
                                       const MCAsmLayout &Layout) {
+  const MCSymbol &OrigSymbol = OrigData.getSymbol();
   MCSymbolData *Data = &OrigData;
   if (Data->isCommon() && Data->isExternal())
     return Data->getCommonAlignment();
@@ -512,8 +514,8 @@ uint64_t ELFObjectWriter::SymbolValue(MCSymbolData &OrigData,
     }
   }
 
-  if ((Data && Data->getFlags() & ELF_Other_ThumbFunc) ||
-      OrigData.getFlags() & ELF_Other_ThumbFunc)
+  const MCAssembler &Asm = Layout.getAssembler();
+  if (Asm.isThumbFunc(&OrigSymbol))
     Res |= 1;
 
   if (!Symbol || !Symbol->isInSection())
@@ -641,8 +643,6 @@ void ELFObjectWriter::WriteSymbol(SymbolTableWriter &Writer, ELFSymbolData &MSD,
     BaseSD = &Layout.getAssembler().getSymbolData(*Base);
     Type = mergeTypeForSet(Type, MCELF::GetType(*BaseSD));
   }
-  if (OrigData.getFlags() & ELF_Other_ThumbFunc)
-    Type = ELF::STT_FUNC;
   uint8_t Info = (Binding << ELF_STB_Shift) | (Type << ELF_STT_Shift);
 
   // Other and Visibility share the same byte with Visibility using the lower
@@ -737,7 +737,8 @@ void ELFObjectWriter::WriteSymbolTable(MCDataFragment *SymtabF,
 // It is always valid to create a relocation with a symbol. It is preferable
 // to use a relocation with a section if that is possible. Using the section
 // allows us to omit some local symbols from the symbol table.
-bool ELFObjectWriter::shouldRelocateWithSymbol(const MCSymbolRefExpr *RefA,
+bool ELFObjectWriter::shouldRelocateWithSymbol(const MCAssembler &Asm,
+                                               const MCSymbolRefExpr *RefA,
                                                const MCSymbolData *SD,
                                                uint64_t C,
                                                unsigned Type) const {
@@ -825,7 +826,7 @@ bool ELFObjectWriter::shouldRelocateWithSymbol(const MCSymbolRefExpr *RefA,
   // bit. With a symbol that is done by just having the symbol have that bit
   // set, so we would lose the bit if we relocated with the section.
   // FIXME: We could use the section but add the bit to the relocation value.
-  if (SD->getFlags() & ELF_Other_ThumbFunc)
+  if (Asm.isThumbFunc(&Sym))
     return true;
 
   if (TargetObjectWriter->needsRelocateWithSymbol(Type))
@@ -887,7 +888,7 @@ void ELFObjectWriter::RecordRelocation(const MCAssembler &Asm,
   const MCSymbolData *SymAD = SymA ? &Asm.getSymbolData(*SymA) : nullptr;
 
   unsigned Type = GetRelocType(Target, Fixup, IsPCRel);
-  bool RelocateWithSymbol = shouldRelocateWithSymbol(RefA, SymAD, C, Type);
+  bool RelocateWithSymbol = shouldRelocateWithSymbol(Asm, RefA, SymAD, C, Type);
   if (!RelocateWithSymbol && SymA && !SymA->isUndefined())
     C += Layout.getSymbolOffset(SymAD);
 

lib/MC/MCAssembler.cpp

@@ -323,6 +323,31 @@ void MCAssembler::reset() {
   getLOHContainer().reset();
 }
 
+bool MCAssembler::isThumbFunc(const MCSymbol *Symbol) const {
+  if (ThumbFuncs.count(Symbol))
+    return true;
+
+  if (!Symbol->isVariable())
+    return false;
+
+  // FIXME: It looks like gas support some cases of the form "foo + 2". It
+  // is not clear if that is a bug or a feature.
+  const MCExpr *Expr = Symbol->getVariableValue();
+  const MCSymbolRefExpr *Ref = dyn_cast<MCSymbolRefExpr>(Expr);
+  if (!Ref)
+    return false;
+
+  if (Ref->getKind() != MCSymbolRefExpr::VK_None)
+    return false;
+
+  const MCSymbol &Sym = Ref->getSymbol();
+  if (!isThumbFunc(&Sym))
+    return false;
+
+  ThumbFuncs.insert(Symbol); // Cache it.
+  return true;
+}
+
 bool MCAssembler::isSymbolLinkerVisible(const MCSymbol &Symbol) const {
   // Non-temporary labels should always be visible to the linker.
   if (!Symbol.isTemporary())

lib/MC/MCMachOStreamer.cpp

@@ -237,10 +237,6 @@ void MCMachOStreamer::EmitThumbFunc(MCSymbol *Symbol) {
   // Remember that the function is a thumb function. Fixup and relocation
   // values will need adjusted.
   getAssembler().setIsThumbFunc(Symbol);
-
-  // Mark the thumb bit on the symbol.
-  MCSymbolData &SD = getAssembler().getOrCreateSymbolData(*Symbol);
-  SD.setFlags(SD.getFlags() | SF_ThumbFunc);
 }
 
 bool MCMachOStreamer::EmitSymbolAttribute(MCSymbol *Symbol,

lib/MC/MachObjectWriter.cpp

@@ -351,6 +351,9 @@ void MachObjectWriter::WriteNlist(MachSymbolData &MSD,
     }
   }
 
+  if (Layout.getAssembler().isThumbFunc(&Symbol))
+    Flags |= SF_ThumbFunc;
+
   // struct nlist (12 bytes)
 
   Write32(MSD.StringIndex);

lib/Target/ARM/MCTargetDesc/ARMELFStreamer.cpp

@@ -609,12 +609,8 @@ private:
   }
 
   void EmitThumbFunc(MCSymbol *Func) override {
-    // FIXME: Anything needed here to flag the function as thumb?
-
     getAssembler().setIsThumbFunc(Func);
-
-    MCSymbolData &SD = getAssembler().getOrCreateSymbolData(*Func);
-    SD.setFlags(SD.getFlags() | ELF_Other_ThumbFunc);
+    EmitSymbolAttribute(Func, MCSA_ELF_TypeFunction);
   }
 
   // Helper functions for ARM exception handling directives

lib/Target/ARM/MCTargetDesc/ARMMachObjectWriter.cpp

@@ -206,11 +206,11 @@ RecordARMScatteredHalfRelocation(MachObjectWriter *Writer,
     // The thumb bit shouldn't be set in the 'other-half' bit of the
     // relocation, but it will be set in FixedValue if the base symbol
     // is a thumb function. Clear it out here.
-    if (A_SD->getFlags() & SF_ThumbFunc)
+    if (Asm.isThumbFunc(A))
       FixedValue &= 0xfffffffe;
     break;
   case ARM::fixup_t2_movt_hi16:
-    if (A_SD->getFlags() & SF_ThumbFunc)
+    if (Asm.isThumbFunc(A))
       FixedValue &= 0xfffffffe;
     MovtBit = 1;
     // Fallthrough

test/MC/ARM/thumb_set.s

@@ -14,6 +14,9 @@ arm_func:
 
 	.thumb_set alias_arm_func, arm_func
 
+        alias_arm_func2 = alias_arm_func
+        alias_arm_func3 = alias_arm_func2
+
 @ ASM: .thumb_set alias_arm_func, arm_func
 
 	.thumb
@@ -64,6 +67,18 @@ beta:
 @ CHECK:   Type: Function
 @ CHECK: }
 
+@ CHECK: Symbol {
+@ CHECK:   Name: alias_arm_func2
+@ CHECK:   Value: 0x1
+@ CHECK:   Type: Function
+@ CHECK: }
+
+@ CHECK: Symbol {
+@ CHECK:   Name: alias_arm_func3
+@ CHECK:   Value: 0x1
+@ CHECK:   Type: Function
+@ CHECK: }
+
 @ CHECK: Symbol {
 @ CHECK:   Name: alias_defined_data
 @ CHECK:   Value: 0x5