[patch] Create a MCSymbolELF
Rafael EspĂndola
rafael.espindola at gmail.com
Mon Jun 1 15:56:02 PDT 2015
>> -MCSymbol *MCContext::getOrCreateSectionSymbol(const MCSectionELF &Section) {
>> - MCSymbol *&Sym = SectionSymbols[&Section];
>> +MCSymbolELF *MCContext::getOrCreateSectionSymbol(const MCSectionELF &Section) {
>> + MCSymbolELF *&Sym = SectionSymbols[&Section];
>> if (Sym)
>> return Sym;
>>
>> @@ -128,12 +128,12 @@ MCSymbol *MCContext::getOrCreateSectionSymbol(const MCSectionELF &Section) {
>>
>> MCSymbol *&OldSym = Symbols[Name];
>> if (OldSym && OldSym->isUndefined()) {
>> - Sym = OldSym;
>> - return OldSym;
>> + Sym = cast<MCSymbolELF>(OldSym);
>> + return Sym;
>> }
>>
>> auto NameIter = UsedNames.insert(std::make_pair(Name, true)).first;
>> - Sym = new (*this) MCSymbol(&*NameIter, /*isTemporary*/ false);
>> + Sym = new (*this) MCSymbolELF(&*NameIter, /*isTemporary*/ false);
>
> It'd be nice to `assert(isELF())` at the top of this function, if that's
> not already there.
It is already passed a MCSectionELF, so that is statically known.
>>
>> if (!OldSym)
>> OldSym = Sym;
>> @@ -163,8 +163,14 @@ MCSymbol *MCContext::CreateSymbol(StringRef Name, bool AlwaysAddSuffix) {
>> if (AllowTemporaryLabels)
>> IsTemporary = Name.startswith(MAI->getPrivateGlobalPrefix());
>>
>> - if (IsTemporary && AlwaysAddSuffix && !UseNamesOnTempLabels)
>> - return new (*this) MCSymbol(nullptr, true);
>> + bool IsELF = MOFI && MOFI->getObjectFileType() == MCObjectFileInfo::IsELF;
>
> Should this be in a function? When is `MOFI` not available?
MOFI is null in llvm-objdump. Running
./bin/llvm-objdump -m -d
~/llvm/llvm/test/tools/llvm-objdump/X86/Inputs/exeThread.macho-x86_64
Fails without this.
>> SmallString<128> NewName = Name;
>> bool AddSuffix = AlwaysAddSuffix;
>> @@ -178,7 +184,11 @@ MCSymbol *MCContext::CreateSymbol(StringRef Name, bool AlwaysAddSuffix) {
>> if (NameEntry.second) {
>> // Ok, we found a name. Have the MCSymbol object itself refer to the copy
>> // of the string that is embedded in the UsedNames entry.
>> - MCSymbol *Result = new (*this) MCSymbol(&*NameEntry.first, IsTemporary);
>> + MCSymbol *Result;
>> + if (IsELF)
>> + Result = new (*this) MCSymbolELF(&*NameEntry.first, IsTemporary);
>> + else
>> + Result = new (*this) MCSymbol(false, &*NameEntry.first, IsTemporary);
>
> Actually, this is somewhat awkward code duplication. It might be better
> to add a private function like this:
>
> MCSymbol *MCContext::createSymbolImpl(
> const StringMapEntry<...> *Name, bool IsTemporary) {
> if (IsELF)
> return new (*this) MCSymbolELF(Name, IsTemporary);
> return new (*this) MCSymbol(/* IsELF */ false, Name, IsTemporary);
> }
Done. An updated patch is attached.
Cheers,
Rafael
-------------- next part --------------
A non-text attachment was scrubbed...
Name: t.patch
Type: text/x-patch
Size: 21732 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150601/78e254f5/attachment.bin>
More information about the llvm-commits
mailing list