Skip to content

Commit 798d7ab

Browse files
committed
Error out of counter splitting if group max is zero
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
1 parent 57f4eba commit 798d7ab

File tree

3 files changed

+28
-4
lines changed

3 files changed

+28
-4
lines changed

include/gpu_performance_api/gpu_perf_api_types.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -170,6 +170,7 @@ typedef enum
170170
kGpaStatusErrorLibAlreadyLoaded = -41,
171171
kGpaStatusErrorOtherSessionActive = -42,
172172
kGpaStatusErrorException = -43,
173+
gGpaInvalidCounterGroupData = -44,
173174
kGpaStatusMin = kGpaStatusErrorException,
174175
kGpaStatusInternal = 256, ///< Status codes used internally within GPUPerfAPI.
175176
} GpaStatus;

source/gpu_perf_api_counter_generator/gpa_counter_scheduler_base.cc

Lines changed: 21 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -308,19 +308,37 @@ GpaStatus GpaCounterSchedulerBase::GetNumRequiredPasses(GpaUInt32* num_required_
308308
// Add the HW groups max's.
309309
for (unsigned int i = 0; i < hw_counters->internal_counter_groups_.size(); ++i)
310310
{
311-
max_counters_per_group.push_back(hw_counters->internal_counter_groups_[i].max_active_discrete_counters);
311+
auto count = hw_counters->internal_counter_groups_[i].max_active_discrete_counters;
312+
if (count == 0)
313+
{
314+
GPA_LOG_DEBUG_ERROR("ERROR: hardware counter group '%s' has zero for max-counters-per-group", hw_counters->internal_counter_groups_[i].name);
315+
return gGpaInvalidCounterGroupData;
316+
}
317+
max_counters_per_group.push_back(count);
312318
}
313319

314320
// Add the Additional groups max's.
315321
for (unsigned int i = 0; i < hw_counters->additional_group_count_; ++i)
316322
{
317-
max_counters_per_group.push_back(hw_counters->additional_groups_[i].max_active_discrete_counters);
323+
auto count = hw_counters->additional_groups_[i].max_active_discrete_counters;
324+
if (count == 0)
325+
{
326+
GPA_LOG_DEBUG_ERROR("ERROR: hardware counter additional group '%s' has zero for max-counters-per-group", hw_counters->additional_groups_[i].name);
327+
return gGpaInvalidCounterGroupData;
328+
}
329+
max_counters_per_group.push_back(count);
318330
}
319331

320332
// TODO: properly handle software groups -- right now, this works because there is only ever a single group defined.
321333
if (sw_counters->group_count_ == 1)
322334
{
323-
max_counters_per_group.push_back(DoGetNumSoftwareCounters());
335+
auto count = DoGetNumSoftwareCounters();
336+
if (count == 0)
337+
{
338+
GPA_LOG_DEBUG_ERROR("ERROR: software counter group has zero for max-counters-per-group");
339+
return gGpaInvalidCounterGroupData;
340+
}
341+
max_counters_per_group.push_back(count);
324342
}
325343

326344
GpaCounterGroupAccessor accessor(hw_counters->internal_counter_groups_,

source/gpu_perf_api_counter_generator/gpa_split_counters_interfaces.h

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,10 +16,10 @@
1616

1717
#ifdef DEBUG_PUBLIC_COUNTER_SPLITTER
1818
#include <sstream>
19-
#include "gpu_perf_api_common/logging.h"
2019
#endif
2120

2221
#include "gpu_perf_api_counter_generator/gpa_derived_counter.h"
22+
#include "gpu_perf_api_common/logging.h"
2323

2424
/// @brief Enum to represent the different SQ shader stages.
2525
enum GpaSqShaderStage
@@ -390,6 +390,11 @@ class IGpaSplitCounters
390390
}
391391

392392
unsigned int group_limit = max_counters_per_group[group_index];
393+
if (group_limit == 0)
394+
{
395+
GPA_LOG_DEBUG_ERROR("ERROR: group(%d) count limit is zero", group_index);
396+
return false;
397+
}
393398

394399
return new_group_used_count <= group_limit;
395400
}

0 commit comments

Comments
 (0)