From ed8a1cd70d091c471f1642030bfcbd969a9160e2 Mon Sep 17 00:00:00 2001 From: Venkateshwar Reddy Kandula Date: Wed, 18 Mar 2026 01:49:15 -0500 Subject: [PATCH 1/4] counters: implement variable bit allocation for dimension encoding Summary: - id_decode.hpp: Added DIM_BIT_LENGTHS[] and DIM_BIT_OFFSETS[] arrays with get_dim_bit_length() and get_dim_bit_offset() helpers. Added static_assert to validate bit allocation. DIMENSION_INSTANCE increased from 6 to 8 bits (256 max value). - evaluate_ast.cpp: Updated perform_reduction(), get_int_encoded_dimensions_from_string(), and perform_selection() to use variable bit allocation. - Test files: Updated to compute per-dimension max values dynamically. --- .../rocprofiler-sdk/counters/evaluate_ast.cpp | 30 ++++-- .../rocprofiler-sdk/counters/id_decode.hpp | 96 +++++++++++++++---- .../counters/tests/dimension.cpp | 35 +++---- .../counters/tests/evaluate_ast_test.cpp | 13 ++- 4 files changed, 124 insertions(+), 50 deletions(-) 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..4ae467ecd123 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 @@ -140,13 +140,15 @@ 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); + // Use variable bit allocation for each dimension + uint64_t dim_bit_length = get_dim_bit_length(dim); + uint64_t dim_bit_offset = get_dim_bit_offset(dim); + int64_t mask_dim = ((1ULL << dim_bit_length) - 1) << dim_bit_offset; rec.id = rec.id | mask_dim; rec.id = rec.id ^ mask_dim; @@ -174,12 +176,16 @@ perform_reduction( } int64_t -get_int_encoded_dimensions_from_string(const std::string& rangeStr) +get_int_encoded_dimensions_from_string(const std::string& rangeStr, + 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; + + // Use variable bit allocation for this dimension + uint64_t bit_length = get_dim_bit_length(dim); + uint64_t max_value = (1ULL << bit_length) - 1; while(std::getline(iss, token, ',')) { @@ -198,13 +204,17 @@ get_int_encoded_dimensions_from_string(const std::string& rangeStr) else { int num = std::stoi(token); - if(num < (1 << bit_length)) + if(static_cast(num) <= max_value) { result |= (1LL << num); } else { - throw std::runtime_error(fmt::format("Dimension value exceeds max allowed.")); + throw std::runtime_error(fmt::format( + "Dimension value {} exceeds max allowed {} for dimension type {}.", + num, + max_value, + static_cast(dim))); } } } @@ -218,9 +228,11 @@ perform_selection(std::mapempty()) 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); + // Use variable bit allocation for this dimension + int64_t encoded_dim_values = get_int_encoded_dimensions_from_string(dim_pair.second, dim_pair.first); + uint64_t dim_bit_length = get_dim_bit_length(dim_pair.first); + uint64_t dim_bit_offset = get_dim_bit_offset(dim_pair.first); + int64_t mask = ((1ULL << dim_bit_length) - 1) << dim_bit_offset; input_array->erase(std::remove_if(input_array->begin(), input_array->end(), 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..6b7de0e832d0 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,59 @@ enum rocprofiler_profile_counter_instance_types ROCPROFILER_DIMENSION_LAST }; +// Variable bit allocation for each dimension type +// Layout (48 bits total for dimensions): +// INSTANCE: 8 bits (supports up to 256 instances) +// 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) +// Total used: 44 bits, 4 bits reserved +constexpr uint8_t DIM_BIT_LENGTHS[ROCPROFILER_DIMENSION_LAST] = { + 0, // DIMENSION_NONE - special case (uses all 48 bits) + 6, // DIMENSION_XCC + 6, // DIMENSION_AID + 6, // DIMENSION_SHADER_ENGINE + 6, // DIMENSION_AGENT + 6, // DIMENSION_SHADER_ARRAY + 6, // DIMENSION_WGP + 8, // DIMENSION_INSTANCE - increased from 6 to 8 bits +}; + +// 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 - special case + 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-43 +}; + +// 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; +} + using DimensionMap = std::unordered_map; @@ -103,27 +156,28 @@ 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"; + // Use variable bit allocation + uint64_t bit_length = get_dim_bit_length(dim); + uint64_t bit_offset = get_dim_bit_offset(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) << ")"; + + // Create mask for this dimension's bits + uint64_t mask = ((1ULL << bit_length) - 1) << bit_offset; + + // Reset bits to 0 for dimension, then set the value + id = (id & ~mask) | (value << bit_offset); } void @@ -152,9 +206,13 @@ 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); + // Use variable bit allocation + uint64_t bit_length = get_dim_bit_length(dim); + uint64_t bit_offset = get_dim_bit_offset(dim); + + // Create mask and extract value + uint64_t mask = ((1ULL << bit_length) - 1) << bit_offset; + 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..3276bdd38ac0 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,9 +82,7 @@ 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_instance_id_t test_id = 0; rocprofiler_counter_id_t test_counter{.handle = 123}; set_counter_in_rec(test_id, test_counter); @@ -98,35 +96,35 @@ TEST(dimension, set_get) check_counter_id(test_id, 321); // Test multiples of i, setting/getting those values across all - // dimensions + // dimensions (using variable bit allocation) for(size_t multi_factor = 1; multi_factor < 7; multi_factor++) { for(size_t i = 1; i < static_cast(ROCPROFILER_DIMENSION_LAST); i++) { auto dim = static_cast(i); + // Get max value for this specific dimension + uint64_t dim_max_val = (1ULL << get_dim_bit_length(dim)) - 1; + 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 +137,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..d32d058e96ff 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,8 +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); + // Use variable bit allocation + uint64_t dim_bit_length = get_dim_bit_length(dim_pair.first); + uint64_t dim_bit_offset = get_dim_bit_offset(dim_pair.first); + int64_t mask = ((1ULL << dim_bit_length) - 1) << dim_bit_offset; for(auto& rec : a) { rec.id = rec.id | mask; @@ -1421,8 +1423,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,7 +1435,10 @@ 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); + // Use variable bit allocation for XCC dimension + uint64_t xcc_bit_length = get_dim_bit_length(ROCPROFILER_DIMENSION_XCC); + uint64_t xcc_bit_offset = get_dim_bit_offset(ROCPROFILER_DIMENSION_XCC); + int64_t mask_dim = ((1ULL << xcc_bit_length) - 1) << xcc_bit_offset; rec.id = rec.id | mask_dim; rec.id = rec.id ^ mask_dim; From e44261c2624f4d2291c757b4348631543d6c55de Mon Sep 17 00:00:00 2001 From: Venkateshwar Reddy Kandula Date: Wed, 18 Mar 2026 12:26:30 -0500 Subject: [PATCH 2/4] Increase DIMENSION_INSTANCE from 8 to 10 bits to support up to 1024 instances - Changed DIMENSION_INSTANCE bit allocation from 8 bits (0-255) to 10 bits (0-1023) - Updated bit layout: 46 bits used, 2 bits reserved - Refactored parse_dimension_selection to use size_t return type - Simplified perform_selection with direct value comparison - Removed unused dim_max_val variable in dimension tests - Used tokenize.hpp utilities for consistent parsing --- .../rocprofiler-sdk/counters/evaluate_ast.cpp | 109 ++++++++++-------- .../rocprofiler-sdk/counters/id_decode.hpp | 27 ++--- .../counters/tests/dimension.cpp | 2 - 3 files changed, 76 insertions(+), 62 deletions(-) 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 4ae467ecd123..b0e4601077bd 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 @@ -175,50 +177,57 @@ perform_reduction( return input_array; } -int64_t -get_int_encoded_dimensions_from_string(const std::string& rangeStr, - rocprofiler_profile_counter_instance_types dim) +/** + * @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; + auto token = sdk::parse::strip(std::string{indexStr}, " \t\n\r"); - // Use variable bit allocation for this dimension - uint64_t bit_length = get_dim_bit_length(dim); - uint64_t max_value = (1ULL << bit_length) - 1; + if(token.empty()) + { + throw std::runtime_error( + fmt::format("Empty dimension selection for dimension type {}.", static_cast(dim))); + } - while(std::getline(iss, token, ',')) + if(token.find(':') != std::string::npos || token.find(',') != std::string::npos) { - token.erase(std::remove_if(token.begin(), token.end(), ::isspace), token.end()); + throw std::runtime_error( + fmt::format("Range or multi-value selection not supported by Dimension API. " + "Only select a single index for each dimension.")); + } - 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(static_cast(num) <= max_value) - { - result |= (1LL << num); - } - else - { - throw std::runtime_error(fmt::format( - "Dimension value {} exceeds max allowed {} for dimension type {}.", - num, - max_value, - static_cast(dim))); - } - } + 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))); } - return result; + + 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* @@ -226,23 +235,29 @@ perform_selection(std::map* input_array) { if(input_array->empty()) return input_array; + for(auto& dim_pair : dimension_map) { - // Use variable bit allocation for this dimension - int64_t encoded_dim_values = get_int_encoded_dimensions_from_string(dim_pair.second, dim_pair.first); - uint64_t dim_bit_length = get_dim_bit_length(dim_pair.first); - uint64_t dim_bit_offset = get_dim_bit_offset(dim_pair.first); - int64_t mask = ((1ULL << dim_bit_length) - 1) << dim_bit_offset; + size_t selected_index = parse_dimension_selection(dim_pair.second, dim_pair.first); + + // Get bit layout for this dimension + uint64_t dim_bit_length = get_dim_bit_length(dim_pair.first); + uint64_t dim_bit_offset = get_dim_bit_offset(dim_pair.first); + int64_t mask = ((1ULL << dim_bit_length) - 1) << dim_bit_offset; 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 = + rocprofiler::counters::rec_to_dim_pos( + rec.id, dim_pair.first); + + // Check if this value matches the selected index + bool should_remove = (dim_val != selected_index); + if(!should_remove) { + // Clear the dimension bits in the record rec.id = rec.id | mask; rec.id = rec.id ^ mask; } 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 6b7de0e832d0..8129ac169ad6 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 @@ -53,41 +53,42 @@ enum rocprofiler_profile_counter_instance_types // Variable bit allocation for each dimension type // Layout (48 bits total for dimensions): -// INSTANCE: 8 bits (supports up to 256 instances) // 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) -// Total used: 44 bits, 4 bits reserved +// 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 - special case (uses all 48 bits) - 6, // DIMENSION_XCC - 6, // DIMENSION_AID - 6, // DIMENSION_SHADER_ENGINE - 6, // DIMENSION_AGENT - 6, // DIMENSION_SHADER_ARRAY - 6, // DIMENSION_WGP - 8, // DIMENSION_INSTANCE - increased from 6 to 8 bits + 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 - special case + 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-43 + 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, + 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 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 3276bdd38ac0..d8b4991e5841 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 @@ -102,8 +102,6 @@ TEST(dimension, set_get) for(size_t i = 1; i < static_cast(ROCPROFILER_DIMENSION_LAST); i++) { auto dim = static_cast(i); - // Get max value for this specific dimension - uint64_t dim_max_val = (1ULL << get_dim_bit_length(dim)) - 1; set_dim_in_rec(test_id, dim, i); check_dim_pos(test_id, dim, i); From ef9f67688b1e62e3174fcf27c272a27b2b2e2ed5 Mon Sep 17 00:00:00 2001 From: Venkateshwar Reddy Kandula Date: Wed, 18 Mar 2026 12:50:14 -0500 Subject: [PATCH 3/4] add helper function get_dim_mask. --- .../rocprofiler-sdk/counters/evaluate_ast.cpp | 24 +++++-------------- .../rocprofiler-sdk/counters/id_decode.hpp | 19 ++++++++------- 2 files changed, 17 insertions(+), 26 deletions(-) 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 b0e4601077bd..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 @@ -147,13 +147,8 @@ perform_reduction( { for(auto dim : _reduce_dimension_set) { - // Use variable bit allocation for each dimension - uint64_t dim_bit_length = get_dim_bit_length(dim); - uint64_t dim_bit_offset = get_dim_bit_offset(dim); - int64_t mask_dim = ((1ULL << dim_bit_length) - 1) << dim_bit_offset; - - 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); } @@ -238,28 +233,21 @@ perform_selection(std::maperase(std::remove_if(input_array->begin(), input_array->end(), [&](rocprofiler_counter_record_t& rec) { size_t dim_val = - rocprofiler::counters::rec_to_dim_pos( - rec.id, dim_pair.first); + rec_to_dim_pos(rec.id, dim_pair.first); - // Check if this value matches the selected index bool should_remove = (dim_val != selected_index); if(!should_remove) { // Clear the dimension bits in the record - rec.id = rec.id | mask; - rec.id = rec.id ^ mask; + 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 8129ac169ad6..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 @@ -105,6 +105,15 @@ 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; @@ -165,18 +174,15 @@ rocprofiler::counters::set_dim_in_rec(rocprofiler_counter_instance_id_t& return; } - // Use variable bit allocation 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) << ")"; - // Create mask for this dimension's bits - uint64_t mask = ((1ULL << bit_length) - 1) << bit_offset; - // Reset bits to 0 for dimension, then set the value id = (id & ~mask) | (value << bit_offset); } @@ -207,12 +213,9 @@ rocprofiler::counters::rec_to_dim_pos(rocprofiler_counter_instance_id_t return id & (MAX_64 >> COUNTER_BIT_LENGTH); } - // Use variable bit allocation - 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); - // Create mask and extract value - uint64_t mask = ((1ULL << bit_length) - 1) << bit_offset; return (id & mask) >> bit_offset; } From 88dedb61c335d084ed4d4a3085ab87cd9d17deef Mon Sep 17 00:00:00 2001 From: Venkateshwar Reddy Kandula Date: Wed, 18 Mar 2026 13:44:40 -0500 Subject: [PATCH 4/4] misc. fixes --- .../counters/tests/dimension.cpp | 6 +++--- .../counters/tests/evaluate_ast_test.cpp | 17 ++++------------- 2 files changed, 7 insertions(+), 16 deletions(-) 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 d8b4991e5841..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,8 +82,8 @@ check_counter_id(rocprofiler_counter_instance_id_t id, uint64_t expected_base_me TEST(dimension, set_get) { using namespace rocprofiler::counters; - 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 @@ -96,7 +96,7 @@ TEST(dimension, set_get) check_counter_id(test_id, 321); // Test multiples of i, setting/getting those values across all - // dimensions (using variable bit allocation) + // dimensions for(size_t multi_factor = 1; multi_factor < 7; multi_factor++) { for(size_t i = 1; i < static_cast(ROCPROFILER_DIMENSION_LAST); i++) 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 d32d058e96ff..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,14 +1312,10 @@ TEST(evatuate_ast, evaluate_select) } for(auto& dim_pair : dims) { - // Use variable bit allocation - uint64_t dim_bit_length = get_dim_bit_length(dim_pair.first); - uint64_t dim_bit_offset = get_dim_bit_offset(dim_pair.first); - int64_t mask = ((1ULL << dim_bit_length) - 1) << dim_bit_offset; + 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; @@ -1435,13 +1431,8 @@ TEST(evaluate_ast, counter_reduction_dimension) std::vector result; for(auto rec : a) { - // Use variable bit allocation for XCC dimension - uint64_t xcc_bit_length = get_dim_bit_length(ROCPROFILER_DIMENSION_XCC); - uint64_t xcc_bit_offset = get_dim_bit_offset(ROCPROFILER_DIMENSION_XCC); - int64_t mask_dim = ((1ULL << xcc_bit_length) - 1) << xcc_bit_offset; - - 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;