This is an archive of the discontinued LLVM Phabricator instance.

Object: Move attribute calculation into RecordStreamer. NFC
ClosedPublic

Authored by vitalybuka on Mar 8 2018, 2:32 PM.

Diff Detail

Repository
rL LLVM

Event Timeline

vitalybuka created this revision.Mar 8 2018, 2:32 PM
vitalybuka added a subscriber: llvm-commits.
espindola added inline comments.Mar 8 2018, 5:36 PM
llvm/include/llvm/MC/MCStreamer.h
530 ↗(On Diff #137658)

I think we have to keep an emit method for this. See https://reviews.llvm.org/D44283.

I am sorry for the current state that symver handling is at. It is basically an 7 year old hack that never got cleaned up.

espindola added inline comments.Mar 9 2018, 10:54 AM
llvm/lib/Object/RecordStreamer.cpp
24 ↗(On Diff #137685)

Starting the name with Get suggests this return the name. Maybe RecordMangledName?

33 ↗(On Diff #137685)

While you are here, could you add Ifuncs? Probably the best way is to just use global_values().

llvm/lib/Object/RecordStreamer.h
71 ↗(On Diff #137685)

Start new functions with a lowercase letter.

vitalybuka marked 3 inline comments as done.

addressing comments and rebase

clang-format

LGTM

Vitaly Buka via Phabricator via llvm-commits
<llvm-commits@lists.llvm.org> writes:

vitalybuka updated this revision to Diff 137808.
vitalybuka added a comment.

clang-format

https://reviews.llvm.org/D44276

Files:

llvm/lib/Object/ModuleSymbolTable.cpp
llvm/lib/Object/RecordStreamer.cpp
llvm/lib/Object/RecordStreamer.h

Index: llvm/lib/Object/RecordStreamer.h

  • llvm/lib/Object/RecordStreamer.h

+++ llvm/lib/Object/RecordStreamer.h
@@ -20,25 +20,35 @@

namespace llvm {

+class GlobalValue;
+class Module;
+
class RecordStreamer : public MCStreamer {
public:

enum State { NeverSeen, Global, Defined, DefinedGlobal, DefinedWeak, Used,
             UndefinedWeak};

private:
+ const Module &M;

StringMap<State> Symbols;
// Map of aliases created by .symver directives, saved so we can update
// their symbol binding after parsing complete. This maps from each
// aliasee to its list of aliases.
DenseMap<const MCSymbol *, std::vector<MCSymbol *>> SymverAliasMap;

+ Mapping from mangled name to GV.
+ StringMap<const GlobalValue *> MangledNameMap;
+
+
/ Get the state recorded for the given symbol.
+ State getSymbolState(const MCSymbol *Sym);
+

void markDefined(const MCSymbol &Symbol);
void markGlobal(const MCSymbol &Symbol, MCSymbolAttr Attribute);
void markUsed(const MCSymbol &Symbol);
void visitUsedSymbol(const MCSymbol &Sym) override;

public:

  • RecordStreamer(MCContext &Context);

+ RecordStreamer(MCContext &Context, const Module &M);

using const_iterator = StringMap<State>::const_iterator;

@@ -56,18 +66,9 @@

/// Record .symver aliases for later processing.
void emitELFSymverDirective(StringRef AliasName,
                            const MCSymbol *Aliasee) override;
  • /// Return the map of .symver aliasee to associated aliases.
  • DenseMap<const MCSymbol *, std::vector<MCSymbol *>> &symverAliases() {
  • return SymverAliasMap;
  • } -
  • /// Get the state recorded for the given symbol.
  • State getSymbolState(const MCSymbol *Sym) {
  • auto SI = Symbols.find(Sym->getName());
  • if (SI == Symbols.end())
  • return NeverSeen;
  • return SI->second;
  • }

+ Ensure ELF .symver aliases get the same binding as the defined symbol
+
they alias with.
+ void emitSymverAttributes();
};

} // end namespace llvm

Index: llvm/lib/Object/RecordStreamer.cpp

  • llvm/lib/Object/RecordStreamer.cpp

+++ llvm/lib/Object/RecordStreamer.cpp
@@ -8,11 +8,29 @@
===----------------------------------------------------------------------===

#include "RecordStreamer.h"
+#include "llvm/IR/Mangler.h"
+#include "llvm/IR/Module.h"
#include "llvm/MC/MCContext.h"
#include "llvm/MC/MCSymbol.h"

using namespace llvm;

+static void initMangledNameMap(const Module &M,
+ StringMap<const GlobalValue *> &MangledNameMap) {
+ The name in the assembler will be mangled, but the name in the IR
+
might not, so we first compute a mapping from mangled name to GV.
+ Mangler Mang;
+ SmallString<64> MangledName;
+ for (const GlobalValue &GV : M.global_values()) {
+ if (!GV.hasName())
+ continue;
+ MangledName.clear();
+ MangledName.reserve(GV.getName().size() + 1);
+ Mang.getNameWithPrefix(MangledName, &GV, /*CannotUsePrivateLabel=*/false);
+ MangledNameMap[MangledName] = &GV;
+ }
+}
+
void RecordStreamer::markDefined(const MCSymbol &Symbol) {

State &S = Symbols[Symbol.getName()];
switch (S) {

@@ -71,7 +89,10 @@

void RecordStreamer::visitUsedSymbol(const MCSymbol &Sym) { markUsed(Sym); }

-RecordStreamer::RecordStreamer(MCContext &Context) : MCStreamer(Context) {}
+RecordStreamer::RecordStreamer(MCContext &Context, const Module &M)
+ : MCStreamer(Context), M(M) {
+ initMangledNameMap(M, MangledNameMap);
+}

RecordStreamer::const_iterator RecordStreamer::begin() {

return Symbols.begin();

@@ -113,10 +134,66 @@

markDefined(*Symbol);

}

+RecordStreamer::State RecordStreamer::getSymbolState(const MCSymbol *Sym) {
+ auto SI = Symbols.find(Sym->getName());
+ if (SI == Symbols.end())
+ return NeverSeen;
+ return SI->second;
+}
+
void RecordStreamer::emitELFSymverDirective(StringRef AliasName,

                                          const MCSymbol *Aliasee) {
MCSymbol *Alias = getContext().getOrCreateSymbol(AliasName);

+ TODO: Handle "@@@". Depending on SymbolAttribute value it needs to be
+
converted into @ or @@.

const MCExpr *Value = MCSymbolRefExpr::create(Aliasee, getContext());
EmitAssignment(Alias, Value);
SymverAliasMap[Aliasee].push_back(Alias);

}
+
+void RecordStreamer::emitSymverAttributes() {
+ Walk all the recorded .symver aliases, and set up the binding
+
for each alias.
+ for (auto &Symver : SymverAliasMap) {
+ const MCSymbol *Aliasee = Symver.first;
+ MCSymbolAttr Attr = MCSA_Invalid;
+
+ First check if the aliasee binding was recorded in the asm.
+ switch (getSymbolState(Aliasee)) {
+ case RecordStreamer::Global:
+ case RecordStreamer::DefinedGlobal:
+ Attr = MCSA_Global;
+ break;
+ case RecordStreamer::UndefinedWeak:
+ case RecordStreamer::DefinedWeak:
+ Attr = MCSA_Weak;
+ break;
+ default:
+ break;
+ }
+
+
If we don't have a symbol attribute from assembly, then check if
+ the aliasee was defined in the IR.
+ if (Attr == MCSA_Invalid) {
+ const auto *GV = M.getNamedValue(Aliasee->getName());
+ if (!GV) {
+ auto MI = MangledNameMap.find(Aliasee->getName());
+ if (MI != MangledNameMap.end())
+ GV = MI->second;
+ else
+ continue;
+ }
+ if (GV->hasExternalLinkage())
+ Attr = MCSA_Global;
+ else if (GV->hasLocalLinkage())
+ Attr = MCSA_Local;
+ else if (GV->isWeakForLinker())
+ Attr = MCSA_Weak;
+ }
+ if (Attr == MCSA_Invalid)
+ continue;
+
Set the detected binding on each alias with this aliasee.
+ for (auto &Alias : Symver.second)
+ EmitSymbolAttribute(Alias, Attr);
+ }
+}

Index: llvm/lib/Object/ModuleSymbolTable.cpp

  • llvm/lib/Object/ModuleSymbolTable.cpp

+++ llvm/lib/Object/ModuleSymbolTable.cpp
@@ -24,7 +24,6 @@
#include "llvm/IR/GlobalAlias.h"
#include "llvm/IR/GlobalValue.h"
#include "llvm/IR/GlobalVariable.h"
-#include "llvm/IR/Mangler.h"
#include "llvm/IR/Module.h"
#include "llvm/MC/MCAsmInfo.h"
#include "llvm/MC/MCContext.h"
@@ -69,81 +68,6 @@

});

}

- Ensure ELF .symver aliases get the same binding as the defined symbol
-
they alias with.
-static void handleSymverAliases(const Module &M, RecordStreamer &Streamer) {

  • if (Streamer.symverAliases().empty())
  • return; -
  • // The name in the assembler will be mangled, but the name in the IR
  • // might not, so we first compute a mapping from mangled name to GV.
  • Mangler Mang;
  • SmallString<64> MangledName;
  • StringMap<const GlobalValue *> MangledNameMap;
  • auto GetMangledName = [&](const GlobalValue &GV) {
  • if (!GV.hasName())
  • return; -
  • MangledName.clear();
  • MangledName.reserve(GV.getName().size() + 1);
  • Mang.getNameWithPrefix(MangledName, &GV, /*CannotUsePrivateLabel=*/false);
  • MangledNameMap[MangledName] = &GV;
  • };
  • for (const Function &F : M)
  • GetMangledName(F);
  • for (const GlobalVariable &GV : M.globals())
  • GetMangledName(GV);
  • for (const GlobalAlias &GA : M.aliases())
  • GetMangledName(GA); -
  • // Walk all the recorded .symver aliases, and set up the binding
  • // for each alias.
  • for (auto &Symver : Streamer.symverAliases()) {
  • const MCSymbol *Aliasee = Symver.first;
  • MCSymbolAttr Attr = MCSA_Invalid; -
  • // First check if the aliasee binding was recorded in the asm.
  • RecordStreamer::State state = Streamer.getSymbolState(Aliasee);
  • switch (state) {
  • case RecordStreamer::Global:
  • case RecordStreamer::DefinedGlobal:
  • Attr = MCSA_Global;
  • break;
  • case RecordStreamer::UndefinedWeak:
  • case RecordStreamer::DefinedWeak:
  • Attr = MCSA_Weak;
  • break;
  • default:
  • break;
  • } -
  • // If we don't have a symbol attribute from assembly, then check if
  • // the aliasee was defined in the IR.
  • if (Attr == MCSA_Invalid) {
  • const auto *GV = M.getNamedValue(Aliasee->getName());
  • if (!GV) {
  • auto MI = MangledNameMap.find(Aliasee->getName());
  • if (MI != MangledNameMap.end())
  • GV = MI->second;
  • else
  • continue;
  • }
  • if (GV->hasExternalLinkage())
  • Attr = MCSA_Global;
  • else if (GV->hasLocalLinkage())
  • Attr = MCSA_Local;
  • else if (GV->isWeakForLinker())
  • Attr = MCSA_Weak;
  • }
  • if (Attr == MCSA_Invalid)
  • continue; -
  • // Set the detected binding on each alias with this aliasee.
  • for (auto &Alias : Symver.second)
  • Streamer.EmitSymbolAttribute(Alias, Attr);
  • }

-}

  • void ModuleSymbolTable::CollectAsmSymbols( const Module &M, function_ref<void(StringRef, BasicSymbolRef::Flags)> AsmSymbol) {

@@ -176,7 +100,7 @@

MCObjectFileInfo MOFI;
MCContext MCCtx(MAI.get(), MRI.get(), &MOFI);
MOFI.InitMCObjectFileInfo(TT, /*PIC*/ false, MCCtx);
  • RecordStreamer Streamer(MCCtx);

+ RecordStreamer Streamer(MCCtx, M);

T->createNullTargetStreamer(Streamer);
 
std::unique_ptr<MemoryBuffer> Buffer(MemoryBuffer::getMemBuffer(InlineAsm));

@@ -195,7 +119,7 @@

if (Parser->Run(false))
  return;
  • handleSymverAliases(M, Streamer);

+ Streamer.emitSymverAttributes();

for (auto &KV : Streamer) {
  StringRef Key = KV.first();

llvm-commits mailing list
llvm-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits

espindola accepted this revision.Mar 9 2018, 2:49 PM

Do you have commit access?

This revision is now accepted and ready to land.Mar 9 2018, 2:49 PM
vitalybuka planned changes to this revision.Mar 9 2018, 2:51 PM

yes, I can commit
but I need to update the patch again.
after Peters comments about deferred processing I don't need MangledName as member

vitalybuka updated this revision to Diff 137854.Mar 9 2018, 3:31 PM

Remove RecordStreamer::MangledNameMap

This revision is now accepted and ready to land.Mar 9 2018, 3:31 PM

LGTM

Vitaly Buka via Phabricator <reviews@reviews.llvm.org> writes:

vitalybuka updated this revision to Diff 137854.
vitalybuka added a comment.
This revision is now accepted and ready to land.

Remove RecordStreamer::MangledNameMap

https://reviews.llvm.org/D44276

Files:

llvm/lib/Object/ModuleSymbolTable.cpp
llvm/lib/Object/RecordStreamer.cpp
llvm/lib/Object/RecordStreamer.h

Index: llvm/lib/Object/RecordStreamer.h

  • llvm/lib/Object/RecordStreamer.h

+++ llvm/lib/Object/RecordStreamer.h
@@ -20,25 +20,32 @@

namespace llvm {

+class GlobalValue;
+class Module;
+
class RecordStreamer : public MCStreamer {
public:

enum State { NeverSeen, Global, Defined, DefinedGlobal, DefinedWeak, Used,
             UndefinedWeak};

private:
+ const Module &M;

StringMap<State> Symbols;
// Map of aliases created by .symver directives, saved so we can update
// their symbol binding after parsing complete. This maps from each
// aliasee to its list of aliases.
  • DenseMap<const MCSymbol *, std::vector<MCSymbol *>> SymverAliasMap;

+ DenseMap<const MCSymbol *, std::vector<StringRef>> SymverAliasMap;
+
+ /// Get the state recorded for the given symbol.
+ State getSymbolState(const MCSymbol *Sym);

void markDefined(const MCSymbol &Symbol);
void markGlobal(const MCSymbol &Symbol, MCSymbolAttr Attribute);
void markUsed(const MCSymbol &Symbol);
void visitUsedSymbol(const MCSymbol &Sym) override;

public:

  • RecordStreamer(MCContext &Context);

+ RecordStreamer(MCContext &Context, const Module &M);

using const_iterator = StringMap<State>::const_iterator;

@@ -56,18 +63,9 @@

/// Record .symver aliases for later processing.
void emitELFSymverDirective(StringRef AliasName,
                            const MCSymbol *Aliasee) override;
  • /// Return the map of .symver aliasee to associated aliases.
  • DenseMap<const MCSymbol *, std::vector<MCSymbol *>> &symverAliases() {
  • return SymverAliasMap;
  • } -
  • /// Get the state recorded for the given symbol.
  • State getSymbolState(const MCSymbol *Sym) {
  • auto SI = Symbols.find(Sym->getName());
  • if (SI == Symbols.end())
  • return NeverSeen;
  • return SI->second;
  • }

+ Emit ELF .symver aliases and ensure they have the same binding as the
+
defined symbol they alias with.
+ void flushSymverDirectives();
};

} // end namespace llvm

Index: llvm/lib/Object/RecordStreamer.cpp

  • llvm/lib/Object/RecordStreamer.cpp

+++ llvm/lib/Object/RecordStreamer.cpp
@@ -8,6 +8,8 @@
===----------------------------------------------------------------------===

#include "RecordStreamer.h"
+#include "llvm/IR/Mangler.h"
+#include "llvm/IR/Module.h"
#include "llvm/MC/MCContext.h"
#include "llvm/MC/MCSymbol.h"

@@ -71,7 +73,8 @@

void RecordStreamer::visitUsedSymbol(const MCSymbol &Sym) { markUsed(Sym); }

-RecordStreamer::RecordStreamer(MCContext &Context) : MCStreamer(Context) {}
+RecordStreamer::RecordStreamer(MCContext &Context, const Module &M)
+ : MCStreamer(Context), M(M) {}

RecordStreamer::const_iterator RecordStreamer::begin() {

return Symbols.begin();

@@ -113,10 +116,82 @@

markDefined(*Symbol);

}

+RecordStreamer::State RecordStreamer::getSymbolState(const MCSymbol *Sym) {
+ auto SI = Symbols.find(Sym->getName());
+ if (SI == Symbols.end())
+ return NeverSeen;
+ return SI->second;
+}
+
void RecordStreamer::emitELFSymverDirective(StringRef AliasName,

const MCSymbol *Aliasee) {
  • MCSymbol *Alias = getContext().getOrCreateSymbol(AliasName);
  • const MCExpr *Value = MCSymbolRefExpr::create(Aliasee, getContext());
  • EmitAssignment(Alias, Value);
  • SymverAliasMap[Aliasee].push_back(Alias);

+ SymverAliasMap[Aliasee].push_back(AliasName);
+}
+
+void RecordStreamer::flushSymverDirectives() {
+ Mapping from mangled name to GV.
+ StringMap<const GlobalValue *> MangledNameMap;
+
The name in the assembler will be mangled, but the name in the IR
+ might not, so we first compute a mapping from mangled name to GV.
+ Mangler Mang;
+ SmallString<64> MangledName;
+ for (const GlobalValue &GV : M.global_values()) {
+ if (!GV.hasName())
+ continue;
+ MangledName.clear();
+ MangledName.reserve(GV.getName().size() + 1);
+ Mang.getNameWithPrefix(MangledName, &GV, /*CannotUsePrivateLabel=*/false);
+ MangledNameMap[MangledName] = &GV;
+ }
+
+
Walk all the recorded .symver aliases, and set up the binding
+ for each alias.
+ for (auto &Symver : SymverAliasMap) {
+ const MCSymbol *Aliasee = Symver.first;
+ MCSymbolAttr Attr = MCSA_Invalid;
+
+
First check if the aliasee binding was recorded in the asm.
+ RecordStreamer::State state = getSymbolState(Aliasee);
+ switch (state) {
+ case RecordStreamer::Global:
+ case RecordStreamer::DefinedGlobal:
+ Attr = MCSA_Global;
+ break;
+ case RecordStreamer::UndefinedWeak:
+ case RecordStreamer::DefinedWeak:
+ Attr = MCSA_Weak;
+ break;
+ default:
+ break;
+ }
+
+ If we don't have a symbol attribute from assembly, then check if
+
the aliasee was defined in the IR.
+ if (Attr == MCSA_Invalid) {
+ const auto *GV = M.getNamedValue(Aliasee->getName());
+ if (!GV) {
+ auto MI = MangledNameMap.find(Aliasee->getName());
+ if (MI != MangledNameMap.end())
+ GV = MI->second;
+ }
+ if (GV) {
+ if (GV->hasExternalLinkage())
+ Attr = MCSA_Global;
+ else if (GV->hasLocalLinkage())
+ Attr = MCSA_Local;
+ else if (GV->isWeakForLinker())
+ Attr = MCSA_Weak;
+ }
+ }
+ Set the detected binding on each alias with this aliasee.
+ for (auto AliasName : Symver.second) {
+ MCSymbol *Alias = getContext().getOrCreateSymbol(AliasName);
+
TODO: Handle "@@@". Depending on SymbolAttribute value it needs to be
+ // converted into @ or @@.
+ const MCExpr *Value = MCSymbolRefExpr::create(Aliasee, getContext());
+ EmitAssignment(Alias, Value);
+ if (Attr != MCSA_Invalid)
+ EmitSymbolAttribute(Alias, Attr);
+ }
+ }
}

Index: llvm/lib/Object/ModuleSymbolTable.cpp

  • llvm/lib/Object/ModuleSymbolTable.cpp

+++ llvm/lib/Object/ModuleSymbolTable.cpp
@@ -24,7 +24,6 @@
#include "llvm/IR/GlobalAlias.h"
#include "llvm/IR/GlobalValue.h"
#include "llvm/IR/GlobalVariable.h"
-#include "llvm/IR/Mangler.h"
#include "llvm/IR/Module.h"
#include "llvm/MC/MCAsmInfo.h"
#include "llvm/MC/MCContext.h"
@@ -69,81 +68,6 @@

});

}

- Ensure ELF .symver aliases get the same binding as the defined symbol
-
they alias with.
-static void handleSymverAliases(const Module &M, RecordStreamer &Streamer) {

  • if (Streamer.symverAliases().empty())
  • return; -
  • // The name in the assembler will be mangled, but the name in the IR
  • // might not, so we first compute a mapping from mangled name to GV.
  • Mangler Mang;
  • SmallString<64> MangledName;
  • StringMap<const GlobalValue *> MangledNameMap;
  • auto GetMangledName = [&](const GlobalValue &GV) {
  • if (!GV.hasName())
  • return; -
  • MangledName.clear();
  • MangledName.reserve(GV.getName().size() + 1);
  • Mang.getNameWithPrefix(MangledName, &GV, /*CannotUsePrivateLabel=*/false);
  • MangledNameMap[MangledName] = &GV;
  • };
  • for (const Function &F : M)
  • GetMangledName(F);
  • for (const GlobalVariable &GV : M.globals())
  • GetMangledName(GV);
  • for (const GlobalAlias &GA : M.aliases())
  • GetMangledName(GA); -
  • // Walk all the recorded .symver aliases, and set up the binding
  • // for each alias.
  • for (auto &Symver : Streamer.symverAliases()) {
  • const MCSymbol *Aliasee = Symver.first;
  • MCSymbolAttr Attr = MCSA_Invalid; -
  • // First check if the aliasee binding was recorded in the asm.
  • RecordStreamer::State state = Streamer.getSymbolState(Aliasee);
  • switch (state) {
  • case RecordStreamer::Global:
  • case RecordStreamer::DefinedGlobal:
  • Attr = MCSA_Global;
  • break;
  • case RecordStreamer::UndefinedWeak:
  • case RecordStreamer::DefinedWeak:
  • Attr = MCSA_Weak;
  • break;
  • default:
  • break;
  • } -
  • // If we don't have a symbol attribute from assembly, then check if
  • // the aliasee was defined in the IR.
  • if (Attr == MCSA_Invalid) {
  • const auto *GV = M.getNamedValue(Aliasee->getName());
  • if (!GV) {
  • auto MI = MangledNameMap.find(Aliasee->getName());
  • if (MI != MangledNameMap.end())
  • GV = MI->second;
  • else
  • continue;
  • }
  • if (GV->hasExternalLinkage())
  • Attr = MCSA_Global;
  • else if (GV->hasLocalLinkage())
  • Attr = MCSA_Local;
  • else if (GV->isWeakForLinker())
  • Attr = MCSA_Weak;
  • }
  • if (Attr == MCSA_Invalid)
  • continue; -
  • // Set the detected binding on each alias with this aliasee.
  • for (auto &Alias : Symver.second)
  • Streamer.EmitSymbolAttribute(Alias, Attr);
  • }

-}

  • void ModuleSymbolTable::CollectAsmSymbols( const Module &M, function_ref<void(StringRef, BasicSymbolRef::Flags)> AsmSymbol) {

@@ -176,7 +100,7 @@

MCObjectFileInfo MOFI;
MCContext MCCtx(MAI.get(), MRI.get(), &MOFI);
MOFI.InitMCObjectFileInfo(TT, /*PIC*/ false, MCCtx);
  • RecordStreamer Streamer(MCCtx);

+ RecordStreamer Streamer(MCCtx, M);

T->createNullTargetStreamer(Streamer);
 
std::unique_ptr<MemoryBuffer> Buffer(MemoryBuffer::getMemBuffer(InlineAsm));

@@ -195,7 +119,7 @@

if (Parser->Run(false))
  return;
  • handleSymverAliases(M, Streamer);

+ Streamer.flushSymverDirectives();

for (auto &KV : Streamer) {
  StringRef Key = KV.first();
This revision was automatically updated to reflect the committed changes.