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.

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

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

33

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

llvm/lib/Object/RecordStreamer.h
71

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.