Skip to content
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

Jump to bottom

[AMD] Fix unhandled profile event in RoctracerProfiler #5252

Merged
merged 1 commit into from
Nov 26, 2024
Merged

[AMD] Fix unhandled profile event in RoctracerProfiler #5252

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
11 changes: 10 additions & 1 deletion third_party/proton/csrc/lib/Profiler/RocTracer/RoctracerProfiler.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ std::shared_ptr<Metric>
convertActivityToMetric(const roctracer_record_t *activity) {
std::shared_ptr<Metric> metric;
switch (activity->kind) {
case kHipVdiCommandTask:
case kHipVdiCommandKernel: {
if (activity->begin_ns < activity->end_ns) {
metric = std::make_shared<KernelMetric>(
Expand Down Expand Up @@ -135,7 +136,7 @@ void processActivity(RoctracerProfiler::CorrIdToExternIdMap &corrIdToExternId,
const roctracer_record_t *record, bool isAPI,
bool isGraph) {
switch (record->kind) {
case 0x11F1: // Task - kernel enqueued by graph launch
case kHipVdiCommandTask:
Copy link
Contributor

Choose a reason for hiding this comment

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

Replacing the magic number is a NFC right? It's just now we have a named variable in the HIP headers?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

NFC? The hip headers are included with Triton so there should not be any issues as this is a drop in replacement as the enum maps to the same number.

Copy link
Contributor

Choose a reason for hiding this comment

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

NFC - no functional change.

case kHipVdiCommandKernel: {
processActivityKernel(corrIdToExternId, externId, dataSet, record, isAPI,
isGraph);
Expand Down Expand Up @@ -169,6 +170,7 @@ std::pair<bool, bool> matchKernelCbId(uint32_t cbId) {
case HIP_API_ID_hipModuleLaunchCooperativeKernel:
case HIP_API_ID_hipModuleLaunchCooperativeKernelMultiDevice:
case HIP_API_ID_hipGraphExecDestroy:
case HIP_API_ID_hipGraphInstantiateWithFlags:
case HIP_API_ID_hipGraphInstantiate: {
isRuntimeApi = true;
break;
Expand Down Expand Up @@ -300,6 +302,13 @@ void RoctracerProfiler::RoctracerProfilerPimpl::apiCallback(
pImpl->StreamToCaptureCount[Stream]++;
break;
}
case HIP_API_ID_hipGraphInstantiateWithFlags: {
hipGraph_t Graph = data->args.hipGraphInstantiateWithFlags.graph;
hipGraphExec_t GraphExec =
*(data->args.hipGraphInstantiateWithFlags.pGraphExec);
pImpl->GraphExecToGraph[GraphExec] = Graph;
break;
}
case HIP_API_ID_hipGraphInstantiate: {
hipGraph_t Graph = data->args.hipGraphInstantiate.graph;
hipGraphExec_t GraphExec = *(data->args.hipGraphInstantiate.pGraphExec);
Expand Down
Loading