Error out of counter splitting if group max is zero #70
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
A counter group having a counter max of zero is invalid and will ultimately result in a hang when we try to split counters into multiple passes. This is one of various scenarios that result in a hang during counter splitting; see GPUOpen-Tools#69 This fixes only that specific scenario. We now check that the group max isn't zero, and if it is, we give up trying to split a public counter's HW counters into multiple passes. We log an error, too. Again, this isn't a comprehensive fix for issue 69. There could be other cases of bad data that result in a hang. Issue 69 should be fixed with a pass cap limit to cover all cases. But this commit still adds value in that it flags the specific invalid GPU counter metadata in addition to avoiding the hang. Change-Id: I56d7d2043ba92c1b6088f0fdd68f5ec844e7b823
PLohrmannAMD
requested changes
Jan 16, 2023
source/gpu_perf_api_counter_generator/gpa_split_counters_interfaces.h
Outdated
Show resolved
Hide resolved
Fixed constant name and log statement
PLohrmannAMD
requested changes
Jan 18, 2023
There was a problem hiding this comment.
A couple nitpicky requests that I should have picked up on in the earlier review - Please remove "ERROR: " from the error messages, capitalize the new first word, and please end the message with a period.
Since they are being reported as a DEBUG_ERROR, the "ERROR:" string is redundant. We chose to let the application decide if/how they want to put additional formatting around the messages being reported.
We actually have a unit test that would confirm that any messages that are output will end with a period, but obviously this particular message is unlikely to be encountered by that test.
Also added missing GPA_ENUM_STRING_VAL for new error code
|
Sure thing. Done. I also realized I neglected to add a GPA_ENUM_STRING_VAL for the new error code |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
A counter group having a counter max of zero is invalid and will ultimately result in a hang when we try to split counters into multiple passes. This is one of various scenarios that result in a hang during counter splitting; see
#69
This fixes only that specific scenario. We now check that the group max isn't zero, and if it is, we give up trying to split a public counter's HW counters into multiple passes. We log an error, too.
Again, this isn't a comprehensive fix for issue 69. There could be other cases of bad data that result in a hang. Issue 69 should be fixed with a pass cap limit to cover all cases. But this commit still adds value in that it flags the specific invalid GPU counter metadata in addition to avoiding the hang.
Change-Id: I56d7d2043ba92c1b6088f0fdd68f5ec844e7b823