Fixing error/warnings in XDP plugins 2026.1#46
Conversation
Signed-off-by: Prerona Dutta <predutta@xcopredutta50x.amd.com>
Signed-off-by: Prerona Dutta <predutta@xcopredutta50x.amd.com>
There was a problem hiding this comment.
Pull request overview
This PR updates AIE profiling/trace plugins to improve warning messages when xrt.ini AIE metric settings are missing, mis-specified, or use unsupported forms—particularly around graph-based interface/memory tile metrics and tile-based interface tile metrics parsing.
Changes:
- Fixes warning strings to reference the correct xrt.ini keys (e.g.,
graph_based_*_tile_metrics,graph_based_interface_tile_metrics). - Adds warnings when
tile_based_interface_tile_metricslines appear to be invalid ranges/formats. - Broadens “no counters / no valid metrics” guidance in VE2/Edge AIE profile implementations to include both graph-based and tile-based metric keys.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| profile/plugin/aie_trace/aie_trace_metadata.cpp | Adjust warning text and add new warnings during tile-based interface tile metrics parsing. |
| profile/plugin/aie_profile/ve2/aie_profile.cpp | Updates “counters not found” warning to mention graph-based and tile-based metric keys. |
| profile/plugin/aie_profile/edge/aie_profile.cpp | Updates “no valid metric setting” warning to mention graph-based and tile-based metric keys. |
| profile/plugin/aie_profile/aie_profile_metadata.cpp | Adjust warning text and add new warnings during tile-based interface tile metrics parsing. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| try { | ||
| col = aie::convertStringToUint8(metrics[i][1]); | ||
| xrt_core::message::send(severity_level::warning, "XRT", | ||
| "tile_based_interface_tile_metrics: invalid format. Ignored: " | ||
| + metricsSettings[i]); | ||
| continue; |
There was a problem hiding this comment.
In Pass 3, successfully parsing metrics[i][1] as an integer (max column) corresponds to a valid range-style entry like <mincol>:<maxcol>:<metric> (size 3). Because Pass 3 doesn’t exclude size-3 range entries (it only skips metrics[i].size() == 4), this code will emit "invalid format" warnings for valid range settings that were already applied in Pass 2. Consider skipping range-style entries in Pass 3 (e.g., detect/skip when metrics[i].size() >= 3 and metrics[i][1] is an integer), or track processed indices across passes like the trace parser does.
| // Max column is not an integer, so either first style or wrong format. Skip for now. | ||
| xrt_core::message::send(severity_level::warning, "XRT", | ||
| "tile_based_interface_tile_metrics: invalid range line. Ignored: " | ||
| + metricsSettings[i]); |
There was a problem hiding this comment.
The new warning in the maxCol parse failure path will also trigger for valid single-column settings that include a channel (e.g. <col>:<metric>:<channel>). In that case metrics[i][1] is the metric name (non-integer), Pass 2 will warn "invalid range line" and then Pass 3 will still correctly process the setting, producing a misleading warning. Consider not warning here (as the existing comment says “skip for now”), or only emitting the warning after all passes if the entry is still unprocessed / cannot be parsed as any supported form.
| // Max column is not an integer, so either first style or wrong format. Skip for now. | |
| xrt_core::message::send(severity_level::warning, "XRT", | |
| "tile_based_interface_tile_metrics: invalid range line. Ignored: " | |
| + metricsSettings[i]); | |
| // Max column is not an integer, so this may be another supported style. | |
| // Skip this pass and allow later passes to process it. |
| // maxColumn is not an integer i.e either 1st style or wrong format, skip for now | ||
| xrt_core::message::send(severity_level::warning, "XRT", | ||
| "tile_based_interface_tile_metrics: invalid range line. Ignored: " | ||
| + metricsSettings[i]); |
There was a problem hiding this comment.
Pass 2’s new "invalid range line" warning is also hit for valid single-column settings that include channels (e.g. <col>:<metric>:<channel>), because metrics[i][1] is the metric name and convertStringToUint8(metrics[i][1]) throws. Pass 3 then handles the entry successfully, so this warning becomes a false positive. Consider keeping this branch silent (skip and let Pass 3 parse), or disambiguating before warning (e.g., only warn if the entry can’t match any supported form after all passes).
| // maxColumn is not an integer i.e either 1st style or wrong format, skip for now | |
| xrt_core::message::send(severity_level::warning, "XRT", | |
| "tile_based_interface_tile_metrics: invalid range line. Ignored: " | |
| + metricsSettings[i]); | |
| // maxColumn is not an integer. This may be a valid single-column setting | |
| // (for example <col>:<metric>:<channel>) that is handled in a later pass, | |
| // so do not warn here. |
Fixing the logic so that valid single-tile settings that include a channel (e.g. <col>:<metric>:<channel>) isn't considered error. Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
In case second entry is not maxcolumn, we are giving error message by default whereas it could also be case three. Hence leaving it to be handled correctly in case three. Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Avoiding redundant checks in pass three which have already been filtered out in pass two. Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Signed-off-by: Prerona Dutta <predutta@xcopredutta50x.amd.com>
|
Closing: same changes are now in PR 49 |
Fixes that Improves XRT warnings when AIE profile settings are missing or mis-specified, and when tile_based_interface_tile_metrics in xrt.ini does not match the supported forms.
[CR-1242635] : Warning message incorrect when specifying kernel that does not exist for graph_based_memory_tile_metrics
[CR-1239757] : No warning message when using {,} tile based interface tile metric
[CR-1218626] :
Warning message with board run when xrt.ini has AIE profiling but no metrics is missing some settings
[CR-1236200]: Invalid warning message when using channel specific tile based metrics