Miscellaneous Updates (const-correctness, logic fixes, etc.) (#126)

* Update lib/rocprofiler/hsa/hsa.cpp

- fix logic for constructing callback_contexts and buffered_contexts arrays

* Update include/rocprofiler/{agent,fwd,pc_sampling}.h

- remove rocprofiler_pc_sampling_config_array_t due to const problems
- update rocprofiler_agent_t to use arrays to const data
- remove redundant rocprofiler_query_pc_sampling_agent_configurations
  - this implementation is quite literally looking up info in the agent struct that was passed

* Update lib/rocprofiler/pc_sampling.cpp

- remove rocprofiler_query_pc_sampling_agent_configurations

* update lib/rocprofiler/agent.cpp

- handle const fields
- make mi200_pc_sampling_config variable static

* Update lib/rocprofiler/tests/agent.cpp

- tweak to pc_sampling_configs offset

* Update samples/pc_sampling

- Update sample to reflect minor tweaks to pc_sampling_configs in rocprofiler_agent_t

* Update CI workflow

- remove 'if: ${{ always() }}'
  - I suspect this is why the jobs do not cancel in progress correctly
This commit is contained in:
Jonathan R. Madsen
2023-10-17 00:39:41 -05:00
committed by GitHub
parent a7a971a247
commit d1518c65b2
11 changed files with 59 additions and 83 deletions
+22 -16
View File
@@ -211,15 +211,20 @@ template <typename MapT, typename Tp>
void
read_property(const MapT& data, const std::string& label, Tp& value)
{
using mutable_type = std::remove_const_t<Tp>;
if constexpr(std::is_enum<Tp>::value)
{
using value_type = std::underlying_type_t<Tp>;
using value_type = std::underlying_type_t<mutable_type>;
// never expect this to be true but it does guard against infinite recursion
static_assert(!std::is_enum<value_type>::value, "Expected non-enum type");
auto value_v = static_cast<value_type>(value);
read_property(data, label, value_v);
value = static_cast<Tp>(value_v);
if constexpr(std::is_const<Tp>::value)
const_cast<mutable_type&>(value) = static_cast<mutable_type>(value_v);
else
value = static_cast<Tp>(value_v);
}
else
{
@@ -258,7 +263,10 @@ read_property(const MapT& data, const std::string& label, Tp& value)
max_value)};
}
value = static_cast<Tp>(local_value);
if constexpr(std::is_const<Tp>::value)
const_cast<mutable_type&>(value) = static_cast<mutable_type>(local_value);
else
value = static_cast<Tp>(local_value);
}
}
@@ -280,7 +288,7 @@ read_topology()
using pc_sampling_config_vec_t = std::vector<rocprofiler_pc_sampling_configuration_t>;
auto mi200_pc_sampling_config = pc_sampling_config_vec_t{
static auto mi200_pc_sampling_config = pc_sampling_config_vec_t{
rocprofiler_pc_sampling_configuration_t{ROCPROFILER_PC_SAMPLING_METHOD_HOST_TRAP,
ROCPROFILER_PC_SAMPLING_UNIT_TIME,
1UL,
@@ -412,12 +420,16 @@ read_topology()
drmClose(drm_fd);
}
constexpr auto gfx90a_version = compute_version(9, 0, 10);
if(agent_info.gfx_target_version >= gfx90a_version)
// TODO(jomadsen): make contingent on whether this process acquired the PC sampling
// device lock
{
agent_info.pc_sampling_configs = rocprofiler_pc_sampling_config_array_t{
mi200_pc_sampling_config.data(), mi200_pc_sampling_config.size()};
constexpr auto gfx90a_version = compute_version(9, 0, 10);
if(agent_info.gfx_target_version >= gfx90a_version)
{
agent_info.pc_sampling_configs = mi200_pc_sampling_config.data();
agent_info.num_pc_sampling_configs = mi200_pc_sampling_config.size();
}
}
}
else if(agent_info.type == ROCPROFILER_AGENT_TYPE_CPU)
@@ -467,16 +479,10 @@ read_topology()
for(uint32_t i = 0; i < agent_info.mem_banks_count; ++i)
{
using heap_type_t = HSA_HEAPTYPE;
using underlying_heap_type_t = std::underlying_type_t<heap_type_t>;
auto subproperties =
read_map(node_path / "mem_banks" / std::to_string(i) / "properties");
auto _heap_type = underlying_heap_type_t{};
read_property(subproperties, "heap_type", _heap_type);
agent_info.mem_banks[i].heap_type = static_cast<heap_type_t>(_heap_type);
read_property(subproperties, "heap_type", agent_info.mem_banks[i].heap_type);
read_property(
subproperties, "size_in_bytes", agent_info.mem_banks[i].size_in_bytes);
read_property(subproperties, "flags", agent_info.mem_banks[i].flags.MemoryProperty);
+7 -11
View File
@@ -198,22 +198,18 @@ hsa_api_impl<Idx>::functor(Args&&... args)
if(itr->callback_tracer)
{
// if the given domain + op is not enabled, skip this context
if(!itr->callback_tracer->domains(info_type::callback_domain_idx,
info_type::operation_idx))
continue;
callback_contexts.emplace_back(
callback_context_data{itr, rocprofiler_callback_tracing_record_t{}});
if(itr->callback_tracer->domains(info_type::callback_domain_idx,
info_type::operation_idx))
callback_contexts.emplace_back(
callback_context_data{itr, rocprofiler_callback_tracing_record_t{}});
}
if(itr->buffered_tracer)
{
// if the given domain + op is not enabled, skip this context
if(!itr->buffered_tracer->domains(info_type::buffered_domain_idx,
info_type::operation_idx))
continue;
buffered_contexts.emplace_back(buffered_context_data{itr});
if(itr->buffered_tracer->domains(info_type::buffered_domain_idx,
info_type::operation_idx))
buffered_contexts.emplace_back(buffered_context_data{itr});
}
}
-9
View File
@@ -48,13 +48,4 @@ rocprofiler_configure_pc_sampling_service(rocprofiler_context_id_t conte
consume_args(context_id, agent, method, unit, interval, buffer_id);
return ROCPROFILER_STATUS_ERROR_NOT_IMPLEMENTED;
}
rocprofiler_status_t
rocprofiler_query_pc_sampling_agent_configurations(rocprofiler_agent_t agent,
rocprofiler_pc_sampling_configuration_t* config,
size_t* config_count)
{
consume_args(agent, config, config_count);
return ROCPROFILER_STATUS_ERROR_NOT_IMPLEMENTED;
}
}
+2 -1
View File
@@ -97,7 +97,8 @@ TEST(rocprofiler_lib, agent_abi)
EXPECT_EQ(offsetof(rocprofiler_agent_t, vendor_name), 256) << msg;
EXPECT_EQ(offsetof(rocprofiler_agent_t, product_name), 264) << msg;
EXPECT_EQ(offsetof(rocprofiler_agent_t, model_name), 272) << msg;
EXPECT_EQ(offsetof(rocprofiler_agent_t, pc_sampling_configs), 280) << msg;
EXPECT_EQ(offsetof(rocprofiler_agent_t, num_pc_sampling_configs), 280) << msg;
EXPECT_EQ(offsetof(rocprofiler_agent_t, pc_sampling_configs), 288) << msg;
// Add test for offset of new field above this. Do NOT change any existing values!
// If a new field is added, increase this value by the size of the new field(s)