Skip to content

Simplify rev opts #352

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 16 commits into from
Jan 7, 2025
Merged

Simplify rev opts #352

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
7d15916
Remove duplicate code in RevOpts
leekillough Dec 23, 2024
6b7a3af
Remove duplicate code in RevOpts
leekillough Dec 23, 2024
3a7f890
Merge remote-tracking branch 'rev/devel' into SimplifyRevOpts
leekillough Dec 23, 2024
4eb3057
Simplify API since the CORES:* logic is always followed by the genera…
leekillough Dec 24, 2024
d208318
add space delimiters; clean up new templates
leekillough Dec 24, 2024
278c9c9
Add header
leekillough Dec 27, 2024
7312fae
change loop iter type to size_t
leekillough Dec 27, 2024
a89f14d
add const to getters; delete copy/assignment
leekillough Dec 27, 2024
884fdd4
optimize string moves to vector and map
leekillough Dec 28, 2024
ceacdb2
Add make_dependent<> to cause lazy evaluation in if-constexpr blocks
leekillough Dec 28, 2024
04f4db7
Add more types to make_dependent<> for more lazy evaluation
leekillough Dec 28, 2024
cb7cacc
simplify make_dependent
leekillough Dec 28, 2024
e2e3116
minor optimizations
leekillough Dec 28, 2024
df1584f
fix comment
leekillough Dec 28, 2024
b6f1a9c
Add const to avoid accidentally modifying reference
leekillough Jan 1, 2025
fddeb8a
Merge remote-tracking branch 'rev/devel' into SimplifyRevOpts
leekillough Jan 6, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion common/include/RevCommon.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ constexpr std::enable_if_t<std::is_integral_v<INT> && std::is_enum_v<ENUM>, INT>
return static_cast<INT>( e );
}

/// Allow non-narrowing int->int cast with enum_int_cast
/// Allow non-narrowing integer->integer cast with safe_static_cast
template<typename INT, typename ENUM, typename = decltype( INT{ std::declval<ENUM>() } )>
constexpr std::enable_if_t<std::is_integral_v<INT> && std::is_integral_v<ENUM>, INT> safe_static_cast( ENUM e ) {
return static_cast<INT>( e );
Expand Down
100 changes: 76 additions & 24 deletions include/RevOpts.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,30 +15,69 @@
#include "SST.h"

// -- Standard Headers
#include "RevCommon.h"
#include <cinttypes>
#include <map>
#include <string>
#include <tuple>
#include <type_traits>
#include <unordered_map>
#include <utility>
#include <vector>

namespace SST::RevCPU {

class RevOpts {

// Queries whether type is a std::vector type
template<typename T>
struct is_vector : std::false_type {};

template<typename T>
struct is_vector<std::vector<T>> : std::true_type {};
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is standard traits programming, where a trait query class is defined as false in general, but then a partial specialization is defined for which it's true for a proper subset of the original template.


// Return a property by looking up its table
// VAL is a universal reference so that either an lvalue reference or an assignable rvalue reference such as std::tie can be used
template<typename MAP, typename VAL>
bool GetProperty( uint32_t Core, const MAP& Map, VAL&& Val ) const {
if constexpr( is_vector<MAP>::value ) {
// If MAP is a vector
return Core < numCores ? Val = Map[Core], true : false;
} else {
// If MAP is a map/unordered_map
auto it = Map.find( Core );
return it != Map.end() ? Val = it->second, true : false;
}
}
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only one of the if constexpr() branches will be taken, and the other will be discarded at compile-time, and will produce no errors.

So we can write one function which operates differently depending on whether MAP is a vector or not.

VAL&& Val is a universal reference, meaning VAL will be of lvalue reference type for lvalue arguments (arguments with a name whose address can be taken) and VAL will be of rvalue reference type for rvalue arguments (arguments with no name or address, which represent temporary expressions which are fleeting and whose resources can be moved instead of deep-copied). Inside GetProperty, Val is an lvalue even if it bound to an rvalue. std::tie() creates an rvalue tuple of references, and can be assigned to even though it is an rvalue, in which case it assigns to its lvalue reference elements.


template<typename VEC>
bool InitPropertyMap( const std::vector<std::string>& Opts, VEC& map );

template<typename VEC>
bool InitPropertyMapCores( const std::vector<std::string>& Opts, VEC& map );

public:
/// RevOpts: options constructor
RevOpts( uint32_t NumCores, uint32_t NumHarts, const int Verbosity );
RevOpts( uint32_t NumCores, uint32_t NumHarts, int Verbosity )
: numCores( NumCores ), numHarts( NumHarts ), verbosity( Verbosity ) {}

/// RevOpts: Disallow copying and assignment
RevOpts( const RevOpts& ) = delete;
RevOpts( RevOpts&& ) = delete;
RevOpts& operator=( const RevOpts& ) = delete;
RevOpts& operator=( RevOpts&& ) = delete;

/// RevOpts: options destructor
~RevOpts() = default;
~RevOpts() = default;

/// RevOpts: retrieve the number of configured cores
uint32_t GetNumCores() { return numCores; }
uint32_t GetNumCores() const { return numCores; }

/// RevOpts: retrieve the number of configured harts per core
uint32_t GetNumHarts() { return numHarts; }
uint32_t GetNumHarts() const { return numHarts; }

/// RevOpts: retrieve the verbosity level
int GetVerbosity() { return verbosity; }
int GetVerbosity() const { return verbosity; }

/// RevOpts: initialize the set of starting addresses
bool InitStartAddrs( const std::vector<std::string>& StartAddrs );
Expand All @@ -59,22 +98,24 @@ class RevOpts {
bool InitPrefetchDepth( const std::vector<std::string>& Depths );

/// RevOpts: retrieve the start address for the target core
bool GetStartAddr( uint32_t Core, uint64_t& StartAddr );
bool GetStartAddr( uint32_t Core, uint64_t& StartAddr ) const { return GetProperty( Core, startAddr, StartAddr ); }
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All of the Get* functions are now implemented in terms of GetProperty().


/// RevOpts: retrieve the start symbol for the target core
bool GetStartSymbol( uint32_t Core, std::string& Symbol );
bool GetStartSymbol( uint32_t Core, std::string& Symbol ) const { return GetProperty( Core, startSym, Symbol ); }

/// RevOpts: retrieve the machine model string for the target core
bool GetMachineModel( uint32_t Core, std::string& MachModel );
bool GetMachineModel( uint32_t Core, std::string& MachModel ) const { return GetProperty( Core, machine, MachModel ); }

/// RevOpts: retrieve instruction table for the target core
bool GetInstTable( uint32_t Core, std::string& Table );
bool GetInstTable( uint32_t Core, std::string& Table ) const { return GetProperty( Core, table, Table ); }

/// RevOpts: retrieve the memory cost range for the target core
bool GetMemCost( uint32_t Core, uint32_t& Min, uint32_t& Max );
bool GetMemCost( uint32_t Core, uint32_t& Min, uint32_t& Max ) const {
return GetProperty( Core, memCosts, std::tie( Min, Max ) );
Copy link
Collaborator Author

@leekillough leekillough Dec 23, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

std::tie( Min, Max ) is passed to GetProperty( ), whose universal reference VAL will be deduced to be an rvalue reference. But it can still be assigned to, and the assignment to Val will extract elements of std::pair or std::tuple into Min and Max.

}

/// RevOpts: retrieve the prefetch depth for the target core
bool GetPrefetchDepth( uint32_t Core, uint32_t& Depth );
bool GetPrefetchDepth( uint32_t Core, uint32_t& Depth ) const { return GetProperty( Core, prefetchDepth, Depth ); }

/// RevOpts: set the argv array
void SetArgs( const SST::Params& params );
Expand All @@ -86,23 +127,34 @@ class RevOpts {
static void splitStr( std::string s, const char* delim, std::vector<std::string>& v ) {
char* ptr = s.data();
char* saveptr = nullptr;
for( v.clear(); auto token = strtok_r( ptr, delim, &saveptr ); ptr = nullptr )
for( v.clear(); char* token = strtok_r( ptr, delim, &saveptr ); ptr = nullptr )
v.push_back( token );
}

private:
uint32_t numCores{}; ///< RevOpts: number of initialized cores
uint32_t numHarts{}; ///< RevOpts: number of harts per core
int verbosity{}; ///< RevOpts: verbosity level

std::unordered_map<uint32_t, uint64_t> startAddr{}; ///< RevOpts: map of core id to starting address
std::unordered_map<uint32_t, std::string> startSym{}; ///< RevOpts: map of core id to starting symbol
std::unordered_map<uint32_t, std::string> machine{}; ///< RevOpts: map of core id to machine model
std::unordered_map<uint32_t, std::string> table{}; ///< RevOpts: map of core id to inst table
std::unordered_map<uint32_t, uint32_t> prefetchDepth{}; ///< RevOpts: map of core id to prefretch depth
std::vector<std::pair<uint32_t, uint32_t>> memCosts{}; ///< RevOpts: vector of memory cost ranges
std::vector<std::string> Argv{}; ///< RevOpts: vector of function arguments
std::vector<std::string> MemDumpRanges{}; ///< RevOpts: vector of function arguments
uint32_t const numCores; ///< RevOpts: number of initialized cores
uint32_t const numHarts; ///< RevOpts: number of harts per core
int const verbosity; ///< RevOpts: verbosity level

// init all the standard options
// -- startAddr = 0x00000000
// -- machine = "G" aka, "IMAFD"
// -- table = internal
// -- memCosts[core] = 0:10
// -- prefetch depth = 16
// -- pipeLine = 5 ???

// clang-format off
std::vector<uint64_t> startAddr{decltype( startAddr )( numCores, 0 )}; ///< RevOpts: starting address
std::vector<std::string> machine{decltype( machine )( numCores, "G" )}; ///< RevOpts: machine model
std::vector<std::string> table{decltype( table )( numCores, "_REV_INTERNAL_" )}; ///< RevOpts: inst table
std::vector<std::pair<uint32_t, uint32_t>> memCosts{decltype( memCosts )( numCores, {0, 10} )}; ///< RevOpts: memory cost range
std::vector<uint32_t> prefetchDepth{decltype( prefetchDepth )( numCores, 16 )}; ///< RevOpts: prefretch depth
// clang-format on
Copy link
Collaborator Author

@leekillough leekillough Dec 23, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All of the vectors which are initialized to default values are initialized at constructor time, after numCores, declared above them, is initialized. The decltype() syntax is syntactic sugar for not having to repeat the type in the far-left column a second time, which we need in order to make direct constructor calls to std::vector rather than initialization-list calls to std::vector, which have a different meaning.


std::unordered_map<uint32_t, std::string> startSym; ///< RevOpts: starting symbol
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because of how RevLoader works, startSym has to remain a std::unordered_map instead of a std::vector. Here it is default-initialized as being empty.

std::vector<std::string> Argv; ///< RevOpts: vector of function arguments
std::vector<std::string> MemDumpRanges; ///< RevOpts: vector of function arguments

}; // class RevOpts

Expand Down
3 changes: 1 addition & 2 deletions src/RevCore.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -247,8 +247,7 @@ std::string RevCore::ExtractMnemonic( const RevInstEntry& Entry ) {
std::string Tmp = Entry.mnemonic;
std::vector<std::string> vstr;
RevOpts::splitStr( Tmp, " ", vstr );

return vstr[0];
return std::move( vstr[0] );
}

bool RevCore::InitTableMapping() {
Expand Down
Loading