[llvm] r207522 - Centralize the handling of the thumb bit.
Rafael Espindola
rafael.espindola at gmail.com
Tue Apr 29 05:46:51 PDT 2014
Author: rafael
Date: Tue Apr 29 07:46:50 2014
New Revision: 207522
URL: http://llvm.org/viewvc/llvm-project?rev=207522&view=rev
Log:
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).
Modified:
llvm/trunk/include/llvm/MC/MCAssembler.h
llvm/trunk/include/llvm/MC/MCELFSymbolFlags.h
llvm/trunk/lib/MC/ELFObjectWriter.cpp
llvm/trunk/lib/MC/MCAssembler.cpp
llvm/trunk/lib/MC/MCMachOStreamer.cpp
llvm/trunk/lib/MC/MachObjectWriter.cpp
llvm/trunk/lib/Target/ARM/MCTargetDesc/ARMELFStreamer.cpp
llvm/trunk/lib/Target/ARM/MCTargetDesc/ARMMachObjectWriter.cpp
llvm/trunk/test/MC/ARM/thumb_set.s
Modified: llvm/trunk/include/llvm/MC/MCAssembler.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/MC/MCAssembler.h?rev=207522&r1=207521&r2=207522&view=diff
==============================================================================
--- llvm/trunk/include/llvm/MC/MCAssembler.h (original)
+++ llvm/trunk/include/llvm/MC/MCAssembler.h Tue Apr 29 07:46:50 2014
@@ -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); }
Modified: llvm/trunk/include/llvm/MC/MCELFSymbolFlags.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/MC/MCELFSymbolFlags.h?rev=207522&r1=207521&r2=207522&view=diff
==============================================================================
--- llvm/trunk/include/llvm/MC/MCELFSymbolFlags.h (original)
+++ llvm/trunk/include/llvm/MC/MCELFSymbolFlags.h Tue Apr 29 07:46:50 2014
@@ -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
Modified: llvm/trunk/lib/MC/ELFObjectWriter.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/MC/ELFObjectWriter.cpp?rev=207522&r1=207521&r2=207522&view=diff
==============================================================================
--- llvm/trunk/lib/MC/ELFObjectWriter.cpp (original)
+++ llvm/trunk/lib/MC/ELFObjectWriter.cpp Tue Apr 29 07:46:50 2014
@@ -215,7 +215,8 @@ class ELFObjectWriter : public MCObjectW
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
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(MC
}
}
- 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(Symbol
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(M
// 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::shouldRelocateWith
// 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(c
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);
Modified: llvm/trunk/lib/MC/MCAssembler.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/MC/MCAssembler.cpp?rev=207522&r1=207521&r2=207522&view=diff
==============================================================================
--- llvm/trunk/lib/MC/MCAssembler.cpp (original)
+++ llvm/trunk/lib/MC/MCAssembler.cpp Tue Apr 29 07:46:50 2014
@@ -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())
Modified: llvm/trunk/lib/MC/MCMachOStreamer.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/MC/MCMachOStreamer.cpp?rev=207522&r1=207521&r2=207522&view=diff
==============================================================================
--- llvm/trunk/lib/MC/MCMachOStreamer.cpp (original)
+++ llvm/trunk/lib/MC/MCMachOStreamer.cpp Tue Apr 29 07:46:50 2014
@@ -237,10 +237,6 @@ void MCMachOStreamer::EmitThumbFunc(MCSy
// 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,
Modified: llvm/trunk/lib/MC/MachObjectWriter.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/MC/MachObjectWriter.cpp?rev=207522&r1=207521&r2=207522&view=diff
==============================================================================
--- llvm/trunk/lib/MC/MachObjectWriter.cpp (original)
+++ llvm/trunk/lib/MC/MachObjectWriter.cpp Tue Apr 29 07:46:50 2014
@@ -351,6 +351,9 @@ void MachObjectWriter::WriteNlist(MachSy
}
}
+ if (Layout.getAssembler().isThumbFunc(&Symbol))
+ Flags |= SF_ThumbFunc;
+
// struct nlist (12 bytes)
Write32(MSD.StringIndex);
Modified: llvm/trunk/lib/Target/ARM/MCTargetDesc/ARMELFStreamer.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/ARM/MCTargetDesc/ARMELFStreamer.cpp?rev=207522&r1=207521&r2=207522&view=diff
==============================================================================
--- llvm/trunk/lib/Target/ARM/MCTargetDesc/ARMELFStreamer.cpp (original)
+++ llvm/trunk/lib/Target/ARM/MCTargetDesc/ARMELFStreamer.cpp Tue Apr 29 07:46:50 2014
@@ -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
Modified: llvm/trunk/lib/Target/ARM/MCTargetDesc/ARMMachObjectWriter.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/ARM/MCTargetDesc/ARMMachObjectWriter.cpp?rev=207522&r1=207521&r2=207522&view=diff
==============================================================================
--- llvm/trunk/lib/Target/ARM/MCTargetDesc/ARMMachObjectWriter.cpp (original)
+++ llvm/trunk/lib/Target/ARM/MCTargetDesc/ARMMachObjectWriter.cpp Tue Apr 29 07:46:50 2014
@@ -206,11 +206,11 @@ RecordARMScatteredHalfRelocation(MachObj
// 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
Modified: llvm/trunk/test/MC/ARM/thumb_set.s
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/MC/ARM/thumb_set.s?rev=207522&r1=207521&r2=207522&view=diff
==============================================================================
--- llvm/trunk/test/MC/ARM/thumb_set.s (original)
+++ llvm/trunk/test/MC/ARM/thumb_set.s Tue Apr 29 07:46:50 2014
@@ -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
@@ -63,6 +66,18 @@ beta:
@ CHECK: Value: 0x1
@ 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
More information about the llvm-commits
mailing list