[AMD] Fix unhandled profile event in RoctracerProfiler #5252
Conversation
|
The reason for the unit tests failing was just an undefined event? I feel like the tests shouldn't fail in that case, just not capture the unknown event? |
| @@ -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: | |||
There was a problem hiding this comment.
Replacing the magic number is a NFC right? It's just now we have a named variable in the HIP headers?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
NFC - no functional change.
It's for an upcoming docker change that bumps ROCm version; see the discussion here #5230 (comment) |
|
Right, I get that but I'm trying to figure out "what" specifically changes when we bump the ROCm version. Did we change the HIP event which tracks graph launches? |
Fixes proton unit tests when upgrading to rocm 6.2 by adding missing event handlers.
Magic number is replaced with the corresponding enum value which was added by upgrading the HIP headers #5077.
New contributor declaration
I am not making a trivial change, such as fixing a typo in a comment.
I have written a PR description following these
rules.
I have run
pre-commit run --from-ref origin/main --to-ref HEAD.Select one of the following.
/testforlittests/unittestfor C++ tests/python/testfor end-to-end testsTests already cover the issue.Select one of the following.
littests.littests I have added follow these best practices,including the "tests should be minimal" section. (Usually running Python code
and using the instructions it generates is not minimal.)