Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -842,7 +842,7 @@ WriteInterceptor(const void* packets,

// SPM is migrated off the per-queue callback registry: call its hook explicitly
// (the other services still flow through signal_callback above).
spm::write_hook(queue,
spm::write_hook(&queue,
kernel_packet,
kernel_id,
dispatch_id,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -344,8 +344,9 @@ set_dispatch_agents(rocprofiler_context_id_t context_id,
for(size_t i = 0; i < num_agents; ++i)
{
const auto* agent = rocprofiler::agent::get_agent(agents[i]);
if(!agent || agent->type != ROCPROFILER_AGENT_TYPE_GPU)
return ROCPROFILER_STATUS_ERROR_AGENT_NOT_FOUND;
if(!agent) return ROCPROFILER_STATUS_ERROR_AGENT_NOT_FOUND;
if(agent->type != ROCPROFILER_AGENT_TYPE_GPU)
return ROCPROFILER_STATUS_ERROR_INVALID_ARGUMENT;
selected.emplace(agents[i]);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ spm_contexts_filter()
} // namespace

void
write_hook(const hsa::Queue& queue,
write_hook(const hsa::Queue* queue,
const hsa::rocprofiler_packet& kernel_packet,
rocprofiler_kernel_id_t kernel_id,
rocprofiler_dispatch_id_t dispatch_id,
Expand All @@ -50,9 +50,11 @@ write_hook(const hsa::Queue& queue,
hsa::inst_pkt_t& inst_pkt,
bool& is_serialized)
{
const auto agent_id = CHECK_NOTNULL(queue.get_agent().get_rocp_agent())->id;

auto active = context::get_active_contexts(spm_contexts_filter());
if(active.empty()) return;

const auto agent_id = CHECK_NOTNULL(CHECK_NOTNULL(queue)->get_agent().get_rocp_agent())->id;

for(const auto* ctx : active)
{
if(!ctx->dispatch_spm->collects_on(agent_id)) continue;
Expand All @@ -61,7 +63,7 @@ write_hook(const hsa::Queue& queue,
{
auto [packet, bSerial] = pre_kernel_call(ctx,
cb,
queue,
*queue,
kernel_packet,
kernel_id,
dispatch_id,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,10 @@ namespace spm
// with the HSA queue controller. Iterates active dispatch_spm contexts and calls
// each callback's pre_kernel_call; appends produced packets to inst_pkt,
// OR-folding each callback's serialize flag into is_serialized.
// queue is nullable on the no-active-context path: the hook returns before it
// dereferences the queue, so tests can exercise the no-op without an HSA runtime.
void
write_hook(const hsa::Queue& queue,
write_hook(const hsa::Queue* queue,
const hsa::rocprofiler_packet& kernel_packet,
rocprofiler_kernel_id_t kernel_id,
rocprofiler_dispatch_id_t dispatch_id,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1123,8 +1123,14 @@ TEST(spm_queue_hooks, stop_context_in_flight_completion_routes_via_hook_path)
}
}

if(!found_spm_agent) GTEST_SKIP() << "SPM unavailable";
FAIL() << "Could not exercise SPM in-flight completion hook path";
registration::set_init_status(1);
registration::finalize();
context::pop_client(1);
set_client_ctx(get_client_ctx());
if(!found_spm_agent)
ROCP_ERROR << "SPM unavailable";
else
FAIL() << "Could not exercise SPM in-flight completion hook path";
}

// Verify that rocprofiler_spm_dispatch_counting_service_set_agents restricts serialization
Expand Down Expand Up @@ -1155,7 +1161,15 @@ TEST(spm_core, set_agents_restricts_collection)
break;
}

if(!target_agent) GTEST_SKIP() << "SPM unavailable";
if(!target_agent)
{
registration::set_init_status(1);
registration::finalize();
context::pop_client(1);
set_client_ctx(get_client_ctx());
ROCP_ERROR << "SPM unavailable";
return;
}

rocprofiler_context_id_t ctx_id{};
ROCPROFILER_CALL(rocprofiler_create_context(&ctx_id), "context creation failed");
Expand Down Expand Up @@ -1228,7 +1242,15 @@ TEST(spm_core, disjoint_contexts_no_conflict)
spm_agents.push_back(rocp_agent->id);
}

if(spm_agents.empty()) GTEST_SKIP() << "SPM unavailable";
if(spm_agents.empty())
{
registration::set_init_status(1);
registration::finalize();
context::pop_client(1);
set_client_ctx(get_client_ctx());
ROCP_ERROR << "SPM unavailable";
return;
}

// ---- Single-agent case: two contexts claiming the same agent must conflict ----
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,4 +53,29 @@ TEST(spm_queue_hooks, exit_hook_skips_when_inst_pkt_has_no_spm_client_id)
nullptr, fq_pkt, sess, packet, inst_pkt, rocprofiler::kernel_dispatch::profiling_time{});
SUCCEED();
}

// write_hook with no active SPM context must be a no-op: it must not append to inst_pkt
// and must not change is_serialized. queue is nullptr because the hook returns before
// dereferencing it when no dispatch_spm context is active (same no-HSA pattern as the
// exit-hook test).
TEST(spm_queue_hooks, write_hook_noop_when_no_context_active)
{
EXPECT_FALSE(rocprofiler::spm::is_any_active());

rocprofiler::hsa::inst_pkt_t inst_pkt;
inst_pkt.emplace_back(std::make_pair(std::make_unique<rocprofiler::hsa::EmptyAQLPacket>(),
rocprofiler::hsa::queue_hooks::COUNTERS_CLIENT_ID));
const auto size_before = inst_pkt.size();

auto fq_pkt = rocprofiler::hsa::rocprofiler_packet{};
rocprofiler_user_data_t user_data = {};
const auto ext_corr_ids = rocprofiler::hsa::queue_info_session_t::external_corr_id_map_t{};
bool is_serialized = true;

rocprofiler::spm::write_hook(
nullptr, fq_pkt, 0, 0, &user_data, ext_corr_ids, nullptr, inst_pkt, is_serialized);

EXPECT_EQ(inst_pkt.size(), size_before);
EXPECT_TRUE(is_serialized);
}
} // namespace
Loading