Skip to content

Commit

Permalink
Clean up some unnecessary mutex guards.
Browse files Browse the repository at this point in the history
These were being used as unreferenced parameters to enforce that
the methods must not be called without holding a mutex, but all
of the methods in question were internal, and the methods were
only exposed through an interface whose entire purpose was to
serialize access to these structures, so expecting the methods
to be accessed under a mutex is reasonable enough.

Reviewed by: blaikie

Differential Revision: http://reviews.llvm.org/D4162

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@211054 91177308-0d34-0410-b5e6-96231b3b80d8
  • Loading branch information
Zachary Turner committed Jun 16, 2014
1 parent 163eb09 commit 4031acb
Show file tree
Hide file tree
Showing 2 changed files with 28 additions and 29 deletions.
8 changes: 4 additions & 4 deletions include/llvm/ExecutionEngine/ExecutionEngine.h
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ namespace object {
}

/// \brief Helper class for helping synchronize access to the global address map
/// table.
/// table. Access to this class should be serialized under a mutex.
class ExecutionEngineState {
public:
struct AddressMapConfig : public ValueMapConfig<const GlobalValue*> {
Expand Down Expand Up @@ -84,19 +84,19 @@ class ExecutionEngineState {
public:
ExecutionEngineState(ExecutionEngine &EE);

GlobalAddressMapTy &getGlobalAddressMap(const MutexGuard &) {
GlobalAddressMapTy &getGlobalAddressMap() {
return GlobalAddressMap;
}

std::map<void*, AssertingVH<const GlobalValue> > &
getGlobalAddressReverseMap(const MutexGuard &) {
getGlobalAddressReverseMap() {
return GlobalAddressReverseMap;
}

/// \brief Erase an entry from the mapping table.
///
/// \returns The address that \p ToUnmap was happed to.
void *RemoveMapping(const MutexGuard &, const GlobalValue *ToUnmap);
void *RemoveMapping(const GlobalValue *ToUnmap);
};

/// \brief Abstract interface for implementation execution of LLVM modules,
Expand Down
49 changes: 24 additions & 25 deletions lib/ExecutionEngine/ExecutionEngine.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -148,8 +148,7 @@ Function *ExecutionEngine::FindFunctionNamed(const char *FnName) {
}


void *ExecutionEngineState::RemoveMapping(const MutexGuard &,
const GlobalValue *ToUnmap) {
void *ExecutionEngineState::RemoveMapping(const GlobalValue *ToUnmap) {
GlobalAddressMapTy::iterator I = GlobalAddressMap.find(ToUnmap);
void *OldVal;

Expand All @@ -171,14 +170,14 @@ void ExecutionEngine::addGlobalMapping(const GlobalValue *GV, void *Addr) {

DEBUG(dbgs() << "JIT: Map \'" << GV->getName()
<< "\' to [" << Addr << "]\n";);
void *&CurVal = EEState.getGlobalAddressMap(locked)[GV];
void *&CurVal = EEState.getGlobalAddressMap()[GV];
assert((!CurVal || !Addr) && "GlobalMapping already established!");
CurVal = Addr;

// If we are using the reverse mapping, add it too.
if (!EEState.getGlobalAddressReverseMap(locked).empty()) {
if (!EEState.getGlobalAddressReverseMap().empty()) {
AssertingVH<const GlobalValue> &V =
EEState.getGlobalAddressReverseMap(locked)[Addr];
EEState.getGlobalAddressReverseMap()[Addr];
assert((!V || !GV) && "GlobalMapping already established!");
V = GV;
}
Expand All @@ -187,41 +186,41 @@ void ExecutionEngine::addGlobalMapping(const GlobalValue *GV, void *Addr) {
void ExecutionEngine::clearAllGlobalMappings() {
MutexGuard locked(lock);

EEState.getGlobalAddressMap(locked).clear();
EEState.getGlobalAddressReverseMap(locked).clear();
EEState.getGlobalAddressMap().clear();
EEState.getGlobalAddressReverseMap().clear();
}

void ExecutionEngine::clearGlobalMappingsFromModule(Module *M) {
MutexGuard locked(lock);

for (Module::iterator FI = M->begin(), FE = M->end(); FI != FE; ++FI)
EEState.RemoveMapping(locked, FI);
EEState.RemoveMapping(FI);
for (Module::global_iterator GI = M->global_begin(), GE = M->global_end();
GI != GE; ++GI)
EEState.RemoveMapping(locked, GI);
EEState.RemoveMapping(GI);
}

void *ExecutionEngine::updateGlobalMapping(const GlobalValue *GV, void *Addr) {
MutexGuard locked(lock);

ExecutionEngineState::GlobalAddressMapTy &Map =
EEState.getGlobalAddressMap(locked);
EEState.getGlobalAddressMap();

// Deleting from the mapping?
if (!Addr)
return EEState.RemoveMapping(locked, GV);
return EEState.RemoveMapping(GV);

void *&CurVal = Map[GV];
void *OldVal = CurVal;

if (CurVal && !EEState.getGlobalAddressReverseMap(locked).empty())
EEState.getGlobalAddressReverseMap(locked).erase(CurVal);
if (CurVal && !EEState.getGlobalAddressReverseMap().empty())
EEState.getGlobalAddressReverseMap().erase(CurVal);
CurVal = Addr;

// If we are using the reverse mapping, add it too.
if (!EEState.getGlobalAddressReverseMap(locked).empty()) {
if (!EEState.getGlobalAddressReverseMap().empty()) {
AssertingVH<const GlobalValue> &V =
EEState.getGlobalAddressReverseMap(locked)[Addr];
EEState.getGlobalAddressReverseMap()[Addr];
assert((!V || !GV) && "GlobalMapping already established!");
V = GV;
}
Expand All @@ -232,25 +231,25 @@ void *ExecutionEngine::getPointerToGlobalIfAvailable(const GlobalValue *GV) {
MutexGuard locked(lock);

ExecutionEngineState::GlobalAddressMapTy::iterator I =
EEState.getGlobalAddressMap(locked).find(GV);
return I != EEState.getGlobalAddressMap(locked).end() ? I->second : nullptr;
EEState.getGlobalAddressMap().find(GV);
return I != EEState.getGlobalAddressMap().end() ? I->second : nullptr;
}

const GlobalValue *ExecutionEngine::getGlobalValueAtAddress(void *Addr) {
MutexGuard locked(lock);

// If we haven't computed the reverse mapping yet, do so first.
if (EEState.getGlobalAddressReverseMap(locked).empty()) {
if (EEState.getGlobalAddressReverseMap().empty()) {
for (ExecutionEngineState::GlobalAddressMapTy::iterator
I = EEState.getGlobalAddressMap(locked).begin(),
E = EEState.getGlobalAddressMap(locked).end(); I != E; ++I)
EEState.getGlobalAddressReverseMap(locked).insert(std::make_pair(
I = EEState.getGlobalAddressMap().begin(),
E = EEState.getGlobalAddressMap().end(); I != E; ++I)
EEState.getGlobalAddressReverseMap().insert(std::make_pair(
I->second, I->first));
}

std::map<void *, AssertingVH<const GlobalValue> >::iterator I =
EEState.getGlobalAddressReverseMap(locked).find(Addr);
return I != EEState.getGlobalAddressReverseMap(locked).end() ? I->second : nullptr;
EEState.getGlobalAddressReverseMap().find(Addr);
return I != EEState.getGlobalAddressReverseMap().end() ? I->second : nullptr;
}

namespace {
Expand Down Expand Up @@ -557,7 +556,7 @@ void *ExecutionEngine::getPointerToGlobal(const GlobalValue *GV) {
return getPointerToFunction(F);

MutexGuard locked(lock);
if (void *P = EEState.getGlobalAddressMap(locked)[GV])
if (void *P = EEState.getGlobalAddressMap()[GV])
return P;

// Global variable might have been added since interpreter started.
Expand All @@ -567,7 +566,7 @@ void *ExecutionEngine::getPointerToGlobal(const GlobalValue *GV) {
else
llvm_unreachable("Global hasn't had an address allocated yet!");

return EEState.getGlobalAddressMap(locked)[GV];
return EEState.getGlobalAddressMap()[GV];
}

/// \brief Converts a Constant* into a GenericValue, including handling of
Expand Down

0 comments on commit 4031acb

Please sign in to comment.