Skip to content

Commit

Permalink
Improve name handling in MachRegister (#1612)
Browse files Browse the repository at this point in the history
* Clean up name caching in MachRegister

This just needs to be a file-scope static. It also doesn't need to be
allocated on the heap or be an ordered container.

* Fix broken transient include

* Have 'name' return ref to const

No need to make a copy.

* Add back error handling for missing name
  • Loading branch information
hainest committed Oct 30, 2023
1 parent 135fa2d commit 362d5f8
Show file tree
Hide file tree
Showing 3 changed files with 15 additions and 21 deletions.
7 changes: 1 addition & 6 deletions common/h/registers/MachRegister.h
Expand Up @@ -34,8 +34,6 @@
#include "Architecture.h"
#include "util.h"

#include <boost/shared_ptr.hpp>
#include <map>
#include <string>

namespace Dyninst {
Expand All @@ -45,9 +43,6 @@ namespace Dyninst {
private:
signed int reg;

typedef std::map<signed int, std::string> NameMap;
static boost::shared_ptr<MachRegister::NameMap> names();

public:
MachRegister();
explicit MachRegister(signed int r);
Expand All @@ -58,7 +53,7 @@ namespace Dyninst {
bool isValid() const;
MachRegisterVal getSubRegValue(const MachRegister& subreg, MachRegisterVal& orig) const;

std::string name() const;
std::string const& name() const;
unsigned int size() const;
bool operator<(const MachRegister& a) const;
bool operator==(const MachRegister& a) const;
Expand Down
28 changes: 13 additions & 15 deletions common/src/registers/MachRegister.C
Expand Up @@ -7,19 +7,20 @@
#include "external/rose/rose-compat.h"

#include <cassert>
#include <unordered_map>

namespace {
std::unordered_map<signed int, std::string> names;
const std::string invalid_reg_name{"<INVALID_REG>"};
}

namespace Dyninst {
boost::shared_ptr<MachRegister::NameMap> MachRegister::names() {
static boost::shared_ptr<MachRegister::NameMap> store =
boost::shared_ptr<MachRegister::NameMap>(new MachRegister::NameMap);
return store;
}

MachRegister::MachRegister() : reg(0) {}

MachRegister::MachRegister(signed int r) : reg(r) {}

MachRegister::MachRegister(signed int r, std::string n) : reg(r) { (*names())[r] = n; }
MachRegister::MachRegister(signed int r, std::string n) : reg(r) { names.emplace(r, std::move(n)); }

unsigned int MachRegister::regClass() const { return reg & 0x00ff0000; }

Expand Down Expand Up @@ -100,16 +101,13 @@ namespace Dyninst {
}
}

std::string MachRegister::name() const {
assert(names() != NULL);
NameMap::const_iterator iter = names()->find(reg);

std::string ret;
if(iter != names()->end()) {
return iter->second;
std::string const& MachRegister::name() const {
auto iter = names.find(reg);
if(iter != names.end()) {
return iter->second;
}
common_parsing_printf(" can't find register with index %x\n", (unsigned int)reg);
return std::string("<INVALID_REG>");
common_parsing_printf("No MachRegister found with value %x\n", static_cast<unsigned int>(reg));
return invalid_reg_name;
}

unsigned int MachRegister::size() const {
Expand Down
1 change: 1 addition & 0 deletions instructionAPI/src/ArchSpecificFormatters.C
Expand Up @@ -12,6 +12,7 @@
#include <string.h>
#include <assert.h>
#include <boost/algorithm/string/join.hpp>
#include <boost/shared_ptr.hpp>
#include "../../common/h/compiler_diagnostics.h"
#include "Architecture.h"
#include "registers/AMDGPU/amdgpu_gfx908_regs.h"
Expand Down

0 comments on commit 362d5f8

Please sign in to comment.