fixes for hw_emu tests failing due to empty device_trace*.csv file#42
fixes for hw_emu tests failing due to empty device_trace*.csv file#42
Conversation
Signed-off-by: Prerona Dutta <predutta@xcopredutta50x.amd.com>
There was a problem hiding this comment.
Pull request overview
This PR addresses HW emulation profiling test failures caused by empty device_trace_*.csv outputs by deferring creation of the device trace CSV writer until PL trace offload initialization succeeds, and by normalizing/validating the Debug.device_trace config value.
Changes:
- Stop creating
device_trace_*.csvwriters from HAL/HW-EMUupdateDevice()paths; create them only after successfulread_trace_init(). - Add
ensureDeviceTraceWriter()plus per-device tracking to prevent empty/duplicate device trace CSV files. - Normalize and validate
Debug.device_trace(case/whitespace) via a helper used consistently across the plugin.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
profile/plugin/device_offload/hw_emu/hw_emu_device_offload_plugin.cpp |
Removes early writer creation in HW emulation path so CSV is not created when trace init fails. |
profile/plugin/device_offload/hal/hal_device_offload_plugin.cpp |
Removes early writer creation in HAL paths; relies on base logic post-trace-init. |
profile/plugin/device_offload/device_offload_plugin.h |
Adds per-device tracking and ensureDeviceTraceWriter() API. |
profile/plugin/device_offload/device_offload_plugin.cpp |
Adds config normalization/validation helper and defers writer creation until read_trace_init() succeeds. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| while (!v.empty() && v.front() == ' ') | ||
| v.erase(0, 1); | ||
| while (!v.empty() && v.back() == ' ') |
There was a problem hiding this comment.
The whitespace trimming in effective_pl_device_trace_mode() only removes ASCII spaces (' '), so values like "fine\t" or Windows-style "fine\r" will be treated as invalid and force device_trace to off. Consider trimming all leading/trailing whitespace (e.g., via std::isspace) to make the validation robust.
| while (!v.empty() && v.front() == ' ') | |
| v.erase(0, 1); | |
| while (!v.empty() && v.back() == ' ') | |
| while (!v.empty() && std::isspace(static_cast<unsigned char>(v.front()))) | |
| v.erase(0, 1); | |
| while (!v.empty() && std::isspace(static_cast<unsigned char>(v.back()))) |
| // std::string data_transfer_trace = xrt_core::config::get_device_trace(); | ||
| std::string data_transfer_trace = effective_pl_device_trace_mode(); | ||
| std::string stall_trace = xrt_core::config::get_stall_trace() ; | ||
|
|
||
| // Set up the hardware trace option |
There was a problem hiding this comment.
configureTraceIP() enables trace based on the config string, but addOffloader() can later force the member device_trace=false when read_trace_init() fails (with device_counters enabled). Since updateDevice() calls configureTraceIP() after addOffloader(), this can still start PL trace even though trace offload init failed. Consider deriving the device-trace/stall-trace enablement from the current device_trace state (or an init-success flag) rather than only the config value.
Fixes for HW emulation tests failing on empty device_trace*.csv
Hardware-emulation profiling tests were failing because device_trace*.csv files were empty . This change adjusts the device offload / trace path so that device trace output is produced (or handled) correctly in HW emulation, restoring the tests.
CR fixed: CR-1264057
The fixes have been tested on vek280 hw_emu design.