dynamic tracing compiler json result support by AdvaitNaik · Pull Request #9760 · Xilinx/XRT

AdvaitNaik · 2026-04-29T23:51:07Z

Problem solved by the commit

Pull in Xilinx/aiebu#278 - dynamic tracing compiler refactor for json output support
Added dtrace_output_json_format (bool, default: false) to control the output format of tracing results.
When set to false (default), results are written to a Python file. When set to true, results are written in JSON format.
Updated the dtrace_log_level default from 1 to 0 and narrow the effective range to 0–2
Updated uc log schema to the latest version

Bug / issue (if any) fixed, which PR introduced the bug, how it was discovered

Added flexibility to dump tracing result in json format while tracing/profiling

How problem was solved, alternative solutions (if any) and why they were rejected

This is a feature enhancement over existing dynamic tracing

Risks (if any) associated the changes in the commit

Low, this applies only when dynamic tracing is enabled

What has been tested and how, request additional testing if necessary

Tested by dumping tracing json result on windows medusa board and the feature works as expected

Documentation impact (if any)

NA

Signed-off-by: Advait Hemant Naik <advanaik@amd.com>

github-actions · 2026-04-29T23:56:30Z

clang-tidy review says "All clean, LGTM! 👍"

Copilot

Pull request overview

Adds support for emitting dynamic tracing compiler results in JSON format (instead of the existing Python output), aligns XRT-side dtrace handle creation with the refactored aiebu dtrace API, and updates the uC log schema text to the latest version.

Changes:

Added Debug.dtrace_output_json_format config to switch dtrace result output between .py and .json.
Refactored dtrace handle creation in xrt_module.cpp to use a dtrace_config_t struct (including output format).
Updated uC log schema message string for fw_state.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File	Description
`src/runtime_src/core/common/uc_log_schema.h`	Updates a schema string for fw_state logging.
`src/runtime_src/core/common/config_reader.h`	Adds config getter for JSON output format; changes dtrace log level default.
`src/runtime_src/core/common/api/xrt_module.cpp`	Passes output format into dtrace creation and uses JSON extension when enabled.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-29T23:56:56Z

    dtrace_util(const std::string& ctrl_file_path, const std::string& map_data,
-                uint32_t log_level)
-      : dtrace_handle(create_dtrace_handle(ctrl_file_path.c_str(), map_data.c_str(), log_level),
-                      destroy_dtrace_handle) {}
+                uint32_t log_level, uint32_t output_fmt)
+      : dtrace_config{ctrl_file_path.c_str(), map_data.c_str(), log_level, output_fmt}
+      , dtrace_handle(create_dtrace_handle(&dtrace_config), destroy_dtrace_handle)
+    {


dtrace_config stores ctrl_file_path.c_str() / map_data.c_str() pointers, but the arguments passed from create_dtrace_util() are a local std::string path and a temporary m_config.dump_buf.to_string(). After the constructor call/assignment completes those underlying string buffers are destroyed, leaving dtrace_config (and potentially the dtrace handle) with dangling pointers. Please persist the strings inside dtrace_util (e.g., std::string ctrl_path_storage; std::string map_data_storage; and set dtrace_config to point to their c_str()), or otherwise ensure create_dtrace_handle() does not retain the pointers beyond the call.

In this flow, create_dtrace_handle does not retain the dtrace_config pointer. It constructs dtrace::control using those inputs, and control takes std::string/const std::string&, so the data is copied during handle creation. That means there is no dangling-pointer use after the constructor call

The code is not clean, the copilot comment shows why it is not structured correctly, please redo. dtrace_config is a data member and it is definitely holding on to stale pointers after construction.

This code still makes no sense. aiebu/.../control.h control ctor takes std::string, you are moving to c_str() only to go back to std::string.

I don't want a constructor body in xrt_module, you have to fix create_dtrace_handle to accept the types it needs. You are faking it by introducing the dtrace_config struct, but then running into problems with what you store in this struct. You could in fact store std::string in config_t, and apply the proper moves in the right places, or you could have create_dtrace_handle accept the types it needs and drop config_t. Moving from std::string -> c_str -> std::string is just weird.

Let's back up and review the changes you made to AIEBU in Xilinx/aiebu#278. For example, I see that control ctor take script_file by value and map_data by const &, why is the former by value, as far as I can see it is not modified in the function. You should request Sonal's review on AIEBU changes and maybe me too (even as I don't really want review AIEBU).

I will accept the xrt_module changes without the dtrace_config struct hack. I will need much time to review the AIEBU changes, but if Sonal approves these I am okay.

the reason there is std::string -> c_str -> std::string is, we want to provide C API, but the implementation in aiebu is cpp and caller is also cpp.
the reason we want to provide C API is there is potential use case in embedded baremetal
so probably here we don't need the config_t, and pass all required to create_handle?

config_t struct was also introduced for future extensibility, adding new parameters for dtrace handle creation then doesn't require changing the API signature

I understand your motivation for config_t, but what does it buy you? You add a new data member to the struct, you change upstream code to populate the member; or you change a function signature and change upstream code to use the new signature. Either way two changes. A config_t struct doesn't buy you anything in this case, that doesn't mean it's wrong or bad, except it is bad in its current use.

stsoe · 2026-04-30T15:19:21Z

    dtrace_util(const std::string& ctrl_file_path, const std::string& map_data,
-                uint32_t log_level)
-      : dtrace_handle(create_dtrace_handle(ctrl_file_path.c_str(), map_data.c_str(), log_level),
-                      destroy_dtrace_handle) {}
+                uint32_t log_level, uint32_t output_fmt)
+      : dtrace_config{ctrl_file_path.c_str(), map_data.c_str(), log_level, output_fmt}
+      , dtrace_handle(create_dtrace_handle(&dtrace_config), destroy_dtrace_handle)
+    {


The code is not clean, the copilot comment shows why it is not structured correctly, please redo. dtrace_config is a data member and it is definitely holding on to stale pointers after construction.

Copilot

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

github-actions · 2026-04-30T17:40:50Z

clang-tidy review says "All clean, LGTM! 👍"

github-actions · 2026-04-30T18:22:30Z

clang-tidy review says "All clean, LGTM! 👍"

Copilot

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Signed-off-by: Advait Hemant Naik <advanaik@amd.com>

github-actions · 2026-04-30T18:40:17Z

clang-tidy review says "All clean, LGTM! 👍"

Copilot

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

AdvaitNaik · 2026-04-30T21:30:14Z

dtrace will switch to C++ API and direct parameter passing to create_dtrace_handle() - dropping dtrace_config_t struct; C API wrapper will be added separately for use case in embedded baremetal.

dtrace json output support

9278ee0

Signed-off-by: Advait Hemant Naik <advanaik@amd.com>

AdvaitNaik requested review from chvamshi-xilinx, Copilot, rbramand-xilinx and xuhz April 29, 2026 23:51

AdvaitNaik requested review from rozumx and stsoe as code owners April 29, 2026 23:51

Copilot started reviewing on behalf of AdvaitNaik April 29, 2026 23:53 View session

Copilot AI reviewed Apr 29, 2026

View reviewed changes

stsoe requested changes Apr 30, 2026

View reviewed changes

AdvaitNaik requested a review from Copilot April 30, 2026 17:35

Copilot started reviewing on behalf of AdvaitNaik April 30, 2026 17:37 View session

Copilot AI reviewed Apr 30, 2026

View reviewed changes

Comment thread src/runtime_src/core/common/api/xrt_module.cpp Outdated

Comment thread src/runtime_src/core/common/api/xrt_module.cpp

AdvaitNaik force-pushed the dtrace_json_output branch from ec9f14b to ac113b2 Compare April 30, 2026 18:17

AdvaitNaik requested a review from Copilot April 30, 2026 18:19

Copilot AI reviewed Apr 30, 2026

View reviewed changes

Comment thread src/runtime_src/core/common/api/xrt_module.cpp Outdated

address comments

824576f

Signed-off-by: Advait Hemant Naik <advanaik@amd.com>

AdvaitNaik force-pushed the dtrace_json_output branch from ac113b2 to 824576f Compare April 30, 2026 18:35

AdvaitNaik requested a review from Copilot April 30, 2026 18:36

Copilot started reviewing on behalf of AdvaitNaik April 30, 2026 18:38 View session

Copilot AI reviewed Apr 30, 2026

View reviewed changes

AdvaitNaik requested a review from stsoe April 30, 2026 18:42

AdvaitNaik closed this Apr 30, 2026

Conversation

AdvaitNaik commented Apr 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Problem solved by the commit

Bug / issue (if any) fixed, which PR introduced the bug, how it was discovered

How problem was solved, alternative solutions (if any) and why they were rejected

Risks (if any) associated the changes in the commit

What has been tested and how, request additional testing if necessary

Documentation impact (if any)

Uh oh!

github-actions Bot commented Apr 29, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Apr 29, 2026

Choose a reason for hiding this comment

Uh oh!

AdvaitNaik Apr 30, 2026

Choose a reason for hiding this comment

Uh oh!

stsoe Apr 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AdvaitNaik Apr 30, 2026

Choose a reason for hiding this comment

Uh oh!

stsoe Apr 30, 2026

Choose a reason for hiding this comment

Uh oh!

xuhz Apr 30, 2026

Choose a reason for hiding this comment

Uh oh!

AdvaitNaik Apr 30, 2026

Choose a reason for hiding this comment

Uh oh!

stsoe Apr 30, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

stsoe Apr 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

github-actions Bot commented Apr 30, 2026

Uh oh!

github-actions Bot commented Apr 30, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

github-actions Bot commented Apr 30, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

AdvaitNaik commented Apr 30, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

AdvaitNaik commented Apr 29, 2026 •

edited

Loading

stsoe Apr 30, 2026 •

edited

Loading

stsoe Apr 30, 2026 •

edited

Loading