fix(rdc): address CPX review feedback and SPX partition-count quirk#8531
Open
brichang wants to merge 1 commit into
Open
Conversation
Resolve the Copilot review comments on PR ROCm#8526: - SmiUtils get_processor_handle_from_id: reject a null out-pointer. - SmiUtils get_num_partition: decode the entity index so both bare flat indices and CPX partition-instance entity indices resolve to the right socket; pass the full entity index from RdcMetricFetcherImpl and RdcGroupSettingsImpl instead of info.device_index. - RdcMetricFetcherImpl RDC_FI_GPU_COUNT: surface an SMI error instead of silently reporting 0 when the flat GPU table is empty. - RdcRocpBase map_entity_to_profiler: also register the encoded partition-instance entity index so CPX partition lookups resolve. - RdcRocpBase.h: correct the stale 10ms default-window comment. Also make get_num_partition robust to drivers that return the current accelerator partition profile with num_partitions set to an invalid sentinel (UINT32_MAX) in SPX: fall back to the flat-table socket count. Validated on MI308X (ROCm 7.2.1), SPX and CPX: rdc_unit_tests and rdc_codec_tests pass; rdci discovery reports 8 (SPX) / 32 (CPX) GPUs; FLOPS_32 attributes to the correct GPU/partition with no entity_to_prof_map lookup errors. Signed-off-by: Brian Chang <brichang@amd.com> Co-authored-by: Cursor <cursoragent@cursor.com>
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Targets
users/dgalants/rdc-cpx-and-profiler-sample-timeto fold into #8526.Summary
Addresses the Copilot review comments on #8526 plus an amdsmi SPX quirk found while validating on MI308X:
SmiUtils::get_processor_handle_from_id: reject a null out-pointer (comment 1).SmiUtils::get_num_partition: decode the entity index so both bare flat indices and CPX partition-instance entity indices resolve to the right socket; callers inRdcMetricFetcherImplandRdcGroupSettingsImplnow pass the full entity index instead ofinfo.device_index(comments 2/4/5).RdcMetricFetcherImplRDC_FI_GPU_COUNT: surface an SMI error instead of silently reporting 0 when the flat GPU table is empty (comment 3).RdcRocpBase::map_entity_to_profiler: also register the encoded partition-instance entity index so CPX partition lookups resolve (comment 6).RdcRocpBase.h: correct the stale "10ms default window" comment (low-confidence note).get_num_partitionrobustness: some drivers return the current accelerator partition profile withnum_partitionsset toUINT32_MAXin SPX even on success; fall back to the flat-table socket count.Test plan
Validated on MI308X (ROCm 7.2.1), built with
-DBUILD_PROFILER=ON -DBUILD_TESTS=ON:rdc_codec_tests: 5/5 pass.rdc_unit_tests(hardware): 7/7 pass in SPX (get_num_partition=1/socket) and CPX (get_num_partition=4/socket).rdci discovery: 8 GPUs in SPX, 32 in CPX.rdci dmonFLOPS_32: attributes only to the loaded GPU (SPX, ~27.5 GFLOP/ms) / partition (CPX partition 0, ~6.8 GFLOP/ms), stable across samples, with noentity_to_prof_maplookup errors.Fixes for #8529.
Signed-off-by: Brian Chang brichang@amd.com