diff --git a/projects/rocprofiler-sdk/source/lib/rocprofiler-sdk/counters/evaluate_ast.cpp b/projects/rocprofiler-sdk/source/lib/rocprofiler-sdk/counters/evaluate_ast.cpp index 3d9cf41a5ab3..68181387e570 100644 --- a/projects/rocprofiler-sdk/source/lib/rocprofiler-sdk/counters/evaluate_ast.cpp +++ b/projects/rocprofiler-sdk/source/lib/rocprofiler-sdk/counters/evaluate_ast.cpp @@ -31,6 +31,8 @@ #include "lib/rocprofiler-sdk/counters/parser/raw_ast.hpp" #include "lib/rocprofiler-sdk/counters/parser/reader.hpp" +#include + #include #include @@ -140,16 +142,13 @@ perform_reduction( } std::unordered_map> rec_groups; - size_t bit_length = DIM_BIT_LENGTH / ROCPROFILER_DIMENSION_LAST; for(auto& rec : *input_array) { for(auto dim : _reduce_dimension_set) { - int64_t mask_dim = (MAX_64 >> (64 - bit_length)) << ((dim - 1) * bit_length); - - rec.id = rec.id | mask_dim; - rec.id = rec.id ^ mask_dim; + uint64_t mask = get_dim_mask(dim); + rec.id = (rec.id | mask) ^ mask; } rec_groups[rec.id].push_back(rec); } @@ -173,42 +172,57 @@ perform_reduction( return input_array; } -int64_t -get_int_encoded_dimensions_from_string(const std::string& rangeStr) +/** + * @brief Parse dimension selection string to get a single selected index. + * + * This function parses a string containing a single dimension index value. + * Only single index selection is supported (e.g., "5"). Range-based selection + * (e.g., "1:4") and multiple values (e.g., "1,2,3") are not supported. + * + * @param indexStr The string containing the dimension index + * @param dim The dimension type for validation + * @return size_t The selected dimension index value + */ +size_t +parse_dimension_selection(const std::string& indexStr, + rocprofiler_profile_counter_instance_types dim) { - int64_t result = 0; - std::istringstream iss(rangeStr); - std::string token; - size_t bit_length = DIM_BIT_LENGTH / ROCPROFILER_DIMENSION_LAST; + auto token = sdk::parse::strip(std::string{indexStr}, " \t\n\r"); - while(std::getline(iss, token, ',')) + if(token.empty()) { - token.erase(std::remove_if(token.begin(), token.end(), ::isspace), token.end()); + throw std::runtime_error( + fmt::format("Empty dimension selection for dimension type {}.", static_cast(dim))); + } - size_t dash_pos = token.find(':'); - if(dash_pos != std::string::npos) - { - throw std::runtime_error( - fmt::format("Range based selection not supported by Dimension API. only select " - "single value for each dimension.")); - int start = std::stoi(token.substr(0, dash_pos)); - int end = std::stoi(token.substr(dash_pos + 1)); - result |= (1LL << std::min(64, end + 1)) - (1LL << std::max(start, 0)); - } - else - { - int num = std::stoi(token); - if(num < (1 << bit_length)) - { - result |= (1LL << num); - } - else - { - throw std::runtime_error(fmt::format("Dimension value exceeds max allowed.")); - } - } + if(token.find(':') != std::string::npos || token.find(',') != std::string::npos) + { + throw std::runtime_error( + fmt::format("Range or multi-value selection not supported by Dimension API. " + "Only select a single index for each dimension.")); } - return result; + + auto bit_length = get_dim_bit_length(dim); + auto max_value = (size_t{1} << bit_length) - 1; + + int num = sdk::parse::from_string(token); + if(num < 0) + { + throw std::runtime_error(fmt::format( + "Dimension value {} is negative for dimension type {}.", num, static_cast(dim))); + } + + auto unum = static_cast(num); + if(unum > max_value) + { + throw std::runtime_error( + fmt::format("Dimension value {} exceeds max allowed {} for dimension type {}.", + num, + max_value, + static_cast(dim))); + } + + return unum; } std::vector* @@ -216,23 +230,24 @@ perform_selection(std::map* input_array) { if(input_array->empty()) return input_array; + for(auto& dim_pair : dimension_map) { - int64_t encoded_dim_values = get_int_encoded_dimensions_from_string(dim_pair.second); - size_t bit_length = DIM_BIT_LENGTH / ROCPROFILER_DIMENSION_LAST; - int64_t mask = (MAX_64 >> (64 - bit_length)) << ((dim_pair.first - 1) * bit_length); + size_t selected_index = parse_dimension_selection(dim_pair.second, dim_pair.first); + uint64_t mask = get_dim_mask(dim_pair.first); input_array->erase(std::remove_if(input_array->begin(), input_array->end(), [&](rocprofiler_counter_record_t& rec) { - bool should_remove = - (encoded_dim_values & - (1 << rocprofiler::counters::rec_to_dim_pos( - rec.id, dim_pair.first))) == 0; + size_t dim_val = + rec_to_dim_pos(rec.id, dim_pair.first); + + bool should_remove = (dim_val != selected_index); + if(!should_remove) { - rec.id = rec.id | mask; - rec.id = rec.id ^ mask; + // Clear the dimension bits in the record + rec.id = (rec.id | mask) ^ mask; } return should_remove; }), diff --git a/projects/rocprofiler-sdk/source/lib/rocprofiler-sdk/counters/id_decode.hpp b/projects/rocprofiler-sdk/source/lib/rocprofiler-sdk/counters/id_decode.hpp index 294532863986..e1e94b964661 100644 --- a/projects/rocprofiler-sdk/source/lib/rocprofiler-sdk/counters/id_decode.hpp +++ b/projects/rocprofiler-sdk/source/lib/rocprofiler-sdk/counters/id_decode.hpp @@ -51,6 +51,69 @@ enum rocprofiler_profile_counter_instance_types ROCPROFILER_DIMENSION_LAST }; +// Variable bit allocation for each dimension type +// Layout (48 bits total for dimensions): +// XCC: 6 bits (supports up to 64) +// AID: 6 bits (supports up to 64) +// SHADER_ENGINE: 6 bits (supports up to 64) +// AGENT: 6 bits (supports up to 64) +// SHADER_ARRAY: 6 bits (supports up to 64) +// WGP: 6 bits (supports up to 64) +// INSTANCE: 10 bits (supports up to 1024 instances) +// Total used: 46 bits, 2 bits reserved +constexpr uint8_t DIM_BIT_LENGTHS[ROCPROFILER_DIMENSION_LAST] = { + 0, // DIMENSION_NONE + 6, // DIMENSION_XCC + 6, // DIMENSION_AID + 6, // DIMENSION_SHADER_ENGINE + 6, // DIMENSION_AGENT + 6, // DIMENSION_SHADER_ARRAY + 6, // DIMENSION_WGP + 10, // DIMENSION_INSTANCE +}; + +// Precomputed bit offsets for each dimension +// Offset is the starting bit position within the 48-bit dimension field +constexpr uint8_t DIM_BIT_OFFSETS[ROCPROFILER_DIMENSION_LAST] = { + 0, // DIMENSION_NONE + 0, // DIMENSION_XCC: bits 0-5 + 6, // DIMENSION_AID: bits 6-11 + 12, // DIMENSION_SHADER_ENGINE: bits 12-17 + 18, // DIMENSION_AGENT: bits 18-23 + 24, // DIMENSION_SHADER_ARRAY: bits 24-29 + 30, // DIMENSION_WGP: bits 30-35 + 36, // DIMENSION_INSTANCE: bits 36-45 +}; + +// Compile-time validation that dimension bit allocation does not exceed 48 bits +static_assert(DIM_BIT_OFFSETS[ROCPROFILER_DIMENSION_INSTANCE] + + DIM_BIT_LENGTHS[ROCPROFILER_DIMENSION_INSTANCE] <= + DIM_BIT_LENGTH, + "Dimension bit allocation exceeds 48-bit limit"); + +// Helper function to get bit length for a dimension +constexpr uint64_t +get_dim_bit_length(rocprofiler_profile_counter_instance_types dim) +{ + return (dim < ROCPROFILER_DIMENSION_LAST) ? DIM_BIT_LENGTHS[dim] : 0; +} + +// Helper function to get bit offset for a dimension +constexpr uint64_t +get_dim_bit_offset(rocprofiler_profile_counter_instance_types dim) +{ + return (dim < ROCPROFILER_DIMENSION_LAST) ? DIM_BIT_OFFSETS[dim] : 0; +} + +// Helper function to get the bitmask for a dimension (shifted to correct position) +constexpr uint64_t +get_dim_mask(rocprofiler_profile_counter_instance_types dim) +{ + uint64_t bit_length = get_dim_bit_length(dim); + uint64_t bit_offset = get_dim_bit_offset(dim); + return ((1ULL << bit_length) - 1) << bit_offset; +} + using DimensionMap = std::unordered_map; @@ -103,27 +166,25 @@ rocprofiler::counters::set_dim_in_rec(rocprofiler_counter_instance_id_t& rocprofiler_profile_counter_instance_types dim, size_t value) { - uint64_t bit_length = DIM_BIT_LENGTH / ROCPROFILER_DIMENSION_LAST; - if(dim == ROCPROFILER_DIMENSION_NONE) { // Set all 48 bits of dimension - id = (id & ~(MAX_64 >> COUNTER_BIT_LENGTH)) | value; - bit_length = DIM_BIT_LENGTH; - } - else - { - uint64_t mask = (MAX_64 >> (BITS_IN_UINT64 - bit_length)) << ((dim - 1) * bit_length); - // Reset bits to 0 for dimension. Does so by getting the bit length as F's then - // shifiting that into the position of dim. Not's that value and then and's it - // with id. - id = (id & ~(mask)); - // Set the value for the dimenstion - id = id | (value << ((dim - 1) * bit_length)); + id = (id & ~(MAX_64 >> COUNTER_BIT_LENGTH)) | value; + CHECK(value <= (MAX_64 >> COUNTER_BIT_LENGTH)) << "Dimension value exceeds max allowed"; + return; } - CHECK(value <= (MAX_64 >> (BITS_IN_UINT64 - bit_length))) - << "Dimension value exceeds max allowed"; + uint64_t bit_length = get_dim_bit_length(dim); + uint64_t bit_offset = get_dim_bit_offset(dim); + uint64_t mask = get_dim_mask(dim); + + CHECK(bit_length > 0) << "Invalid dimension type"; + CHECK(value <= ((1ULL << bit_length) - 1)) + << "Dimension value " << value << " exceeds max allowed for dimension " + << static_cast(dim) << " (max: " << ((1ULL << bit_length) - 1) << ")"; + + // Reset bits to 0 for dimension, then set the value + id = (id & ~mask) | (value << bit_offset); } void @@ -152,9 +213,10 @@ rocprofiler::counters::rec_to_dim_pos(rocprofiler_counter_instance_id_t return id & (MAX_64 >> COUNTER_BIT_LENGTH); } - size_t bit_length = DIM_BIT_LENGTH / ROCPROFILER_DIMENSION_LAST; - id = id & ((MAX_64 >> (BITS_IN_UINT64 - bit_length)) << ((dim - 1) * bit_length)); - return id >> ((dim - 1) * bit_length); + uint64_t bit_offset = get_dim_bit_offset(dim); + uint64_t mask = get_dim_mask(dim); + + return (id & mask) >> bit_offset; } // Counter ID encoding/decoding implementations diff --git a/projects/rocprofiler-sdk/source/lib/rocprofiler-sdk/counters/tests/dimension.cpp b/projects/rocprofiler-sdk/source/lib/rocprofiler-sdk/counters/tests/dimension.cpp index 61fc8644d36b..71538d352b5e 100644 --- a/projects/rocprofiler-sdk/source/lib/rocprofiler-sdk/counters/tests/dimension.cpp +++ b/projects/rocprofiler-sdk/source/lib/rocprofiler-sdk/counters/tests/dimension.cpp @@ -82,10 +82,8 @@ check_counter_id(rocprofiler_counter_instance_id_t id, uint64_t expected_base_me TEST(dimension, set_get) { using namespace rocprofiler::counters; - int64_t max_counter_val = (std::numeric_limits::max() >> - (64 - (DIM_BIT_LENGTH / ROCPROFILER_DIMENSION_LAST))); - rocprofiler_counter_instance_id_t test_id = 0; - rocprofiler_counter_id_t test_counter{.handle = 123}; + auto test_id = rocprofiler_counter_instance_id_t{0}; + auto test_counter = rocprofiler_counter_id_t{.handle = 123}; set_counter_in_rec(test_id, test_counter); // 0x007B000000000000 = decimal counter id 123 << DIM_BIT_LENGTH @@ -104,29 +102,27 @@ TEST(dimension, set_get) for(size_t i = 1; i < static_cast(ROCPROFILER_DIMENSION_LAST); i++) { auto dim = static_cast(i); + set_dim_in_rec(test_id, dim, i); check_dim_pos(test_id, dim, i); set_dim_in_rec(test_id, dim, i * multi_factor); for(size_t j = 1; j < static_cast(ROCPROFILER_DIMENSION_LAST); j++) { if(i == j) continue; - set_dim_in_rec(test_id, - static_cast(j), - max_counter_val); - check_dim_pos(test_id, - static_cast(j), - max_counter_val); + auto other_dim = static_cast(j); + uint64_t other_max_val = (1ULL << get_dim_bit_length(other_dim)) - 1; + set_dim_in_rec(test_id, other_dim, other_max_val); + check_dim_pos(test_id, other_dim, other_max_val); check_dim_pos(test_id, dim, i * multi_factor); } for(size_t j = static_cast(ROCPROFILER_DIMENSION_LAST - 1); j > 0; j--) { if(i == j) continue; - set_dim_in_rec(test_id, - static_cast(j), - max_counter_val); - check_dim_pos( - test_id, (rocprofiler_profile_counter_instance_types) j, max_counter_val); + auto other_dim = static_cast(j); + uint64_t other_max_val = (1ULL << get_dim_bit_length(other_dim)) - 1; + set_dim_in_rec(test_id, other_dim, other_max_val); + check_dim_pos(test_id, other_dim, other_max_val); check_dim_pos(test_id, dim, i * multi_factor); } @@ -139,12 +135,15 @@ TEST(dimension, set_get) for(size_t i = static_cast(ROCPROFILER_DIMENSION_LAST - 1); i > 0; i--) { - auto dim = static_cast(i); + auto dim = static_cast(i); + uint64_t dim_max_val = (1ULL << get_dim_bit_length(dim)) - 1; + set_dim_in_rec(test_id, dim, i * 5); check_dim_pos(test_id, dim, i * 5); set_dim_in_rec(test_id, dim, i * 3); check_dim_pos(test_id, dim, i * 3); - for(size_t j = 1; j < 64; j++) + // Test all values up to the max for this dimension + for(size_t j = 1; j <= dim_max_val; j++) { test_id = 0; set_dim_in_rec(test_id, dim, j); diff --git a/projects/rocprofiler-sdk/source/lib/rocprofiler-sdk/counters/tests/evaluate_ast_test.cpp b/projects/rocprofiler-sdk/source/lib/rocprofiler-sdk/counters/tests/evaluate_ast_test.cpp index 05ec89af428b..9410c341a5d0 100644 --- a/projects/rocprofiler-sdk/source/lib/rocprofiler-sdk/counters/tests/evaluate_ast_test.cpp +++ b/projects/rocprofiler-sdk/source/lib/rocprofiler-sdk/counters/tests/evaluate_ast_test.cpp @@ -1312,12 +1312,10 @@ TEST(evatuate_ast, evaluate_select) } for(auto& dim_pair : dims) { - size_t bit_length = DIM_BIT_LENGTH / ROCPROFILER_DIMENSION_LAST; - int64_t mask = (MAX_64 >> (64 - bit_length)) << ((dim_pair.first - 1) * bit_length); + uint64_t mask = get_dim_mask(dim_pair.first); for(auto& rec : a) { - rec.id = rec.id | mask; - rec.id = rec.id ^ mask; + rec.id = (rec.id | mask) ^ mask; } } return a; @@ -1421,8 +1419,6 @@ TEST(evaluate_ast, counter_reduction_dimension) { using namespace rocprofiler::counters; - size_t bit_length = DIM_BIT_LENGTH / ROCPROFILER_DIMENSION_LAST; - auto get_base_rec_id = [](uint64_t counter_id) { rocprofiler_counter_instance_id_t base_id = 0; set_counter_in_rec(base_id, {.handle = counter_id}); @@ -1435,10 +1431,8 @@ TEST(evaluate_ast, counter_reduction_dimension) std::vector result; for(auto rec : a) { - int64_t mask_dim = (MAX_64 >> (64 - bit_length)) << (bit_length * 0); - - rec.id = rec.id | mask_dim; - rec.id = rec.id ^ mask_dim; + uint64_t mask = get_dim_mask(ROCPROFILER_DIMENSION_XCC); + rec.id = (rec.id | mask) ^ mask; if(groups_dim.find(rec.id) == groups_dim.end()) { groups_dim[rec.id] = rec;