Skip to content

Conversation

@pgschuey
Copy link
Collaborator

@pgschuey pgschuey commented Aug 6, 2025

Problem solved by the commit

XDP needed support for NPU3

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

Added extensive support for NPU3

Risks (if any) associated the changes in the commit

Support affects many files and build environments

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

Simulation of NPU3

Documentation impact (if any)

New events, metric sets, etc.

Paul Schumacher added 30 commits June 12, 2025 17:19
Signed-off-by: Paul Schumacher <schuey@xilinx.com>
Signed-off-by: Paul Schumacher <schuey@xilinx.com>
Signed-off-by: Paul Schumacher <schuey@xilinx.com>
Signed-off-by: Paul Schumacher <schuey@xilinx.com>
Signed-off-by: Paul Schumacher <schuey@xilinx.com>
Signed-off-by: Paul Schumacher <schuey@xilinx.com>
Signed-off-by: Paul Schumacher <schuey@xilinx.com>
Signed-off-by: Paul Schumacher <schuey@xilinx.com>
Signed-off-by: Paul Schumacher <schuey@xilinx.com>
Signed-off-by: Paul Schumacher <schuey@xilinx.com>
Signed-off-by: Paul Schumacher <schuey@xilinx.com>
Signed-off-by: Paul Schumacher <schuey@xilinx.com>
Signed-off-by: Paul Schumacher <schuey@xilinx.com>
Signed-off-by: Paul Schumacher <schuey@xilinx.com>
Signed-off-by: Paul Schumacher <schuey@xilinx.com>
Signed-off-by: Paul Schumacher <schuey@xilinx.com>
Signed-off-by: Paul Schumacher <schuey@xilinx.com>
Signed-off-by: Paul Schumacher <schuey@xilinx.com>
Signed-off-by: Paul Schumacher <schuey@xilinx.com>
Signed-off-by: Paul Schumacher <schuey@xilinx.com>
Signed-off-by: Paul Schumacher <schuey@xilinx.com>
Signed-off-by: Paul Schumacher <schuey@xilinx.com>
Signed-off-by: Paul Schumacher <schuey@xilinx.com>
Signed-off-by: Paul Schumacher <schuey@xilinx.com>
Signed-off-by: Paul Schumacher <schuey@xilinx.com>
Signed-off-by: Paul Schumacher <schuey@xilinx.com>
Signed-off-by: Paul Schumacher <schuey@xilinx.com>
Signed-off-by: Paul Schumacher <schuey@xilinx.com>
Signed-off-by: Paul Schumacher <schuey@xilinx.com>
Signed-off-by: Paul Schumacher <schuey@xilinx.com>
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clang-tidy made some suggestions

uint8_t burstLength;
uint8_t type;

TraceGMIO(uint32_t i, uint8_t col, uint8_t num,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: 6 adjacent parameters of 'TraceGMIO' of convertible types are easily swapped by mistake [bugprone-easily-swappable-parameters]

    TraceGMIO(uint32_t i, uint8_t col, uint8_t num, 
              ^
Additional context

src/runtime_src/xdp/profile/database/static_info/aie_constructs.h:214: the first parameter in the range is 'i'

    TraceGMIO(uint32_t i, uint8_t col, uint8_t num, 
                       ^

src/runtime_src/xdp/profile/database/static_info/aie_constructs.h:215: the last parameter in the range is 't'

              uint8_t stream, uint8_t len, uint8_t t = 0)
                                                   ^

src/runtime_src/xdp/profile/database/static_info/aie_constructs.h:214:

    TraceGMIO(uint32_t i, uint8_t col, uint8_t num, 
              ^

src/runtime_src/xdp/profile/database/static_info/aie_constructs.h:214: 'uint32_t' and 'uint8_t' may be implicitly converted: 'uint32_t' (as 'unsigned int') -> 'uint8_t' (as 'unsigned char'), 'uint8_t' (as 'unsigned char') -> 'uint32_t' (as 'unsigned int')

    TraceGMIO(uint32_t i, uint8_t col, uint8_t num, 
                          ^

configWriter = new AieTraceConfigWriter(configFile.c_str(), deviceID);
writers.push_back(configWriter);
(db->getStaticInfo())
.addOpenedFile(configWriter->getcurrentFileName(),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: no member named 'addOpenedFile' in 'xdp::VPStaticDatabase' [clang-diagnostic-error]

        .addOpenedFile(configWriter->getcurrentFileName(),
         ^

Signed-off-by: Paul Schumacher <schuey@xilinx.com>
Copy link
Collaborator

@stsoe stsoe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rebase against latest master.

jyothees99 and others added 11 commits September 17, 2025 15:48
Signed-off-by: Jyotheeswar Ganne <Jyotheeswar.Ganne@amd.com>
On behalf of Jain <parthash@amd.com>, I, Paul Schumacher <schuey@xilinx.com>, hereby add my Signed-off-by to this commit: 304f91d

Signed-off-by: Paul Schumacher <schuey@xilinx.com>
Signed-off-by: Paul Schumacher <schuey@xilinx.com>
…aie_devel

Signed-off-by: Jain <parthash@amd.com>
… in aie_profile npu3

Signed-off-by: Jyotheeswar Ganne <Jyotheeswar.Ganne@amd.com>
Signed-off-by: Jain <parthash@amd.com>
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clang-tidy made some suggestions

auto hwGen = metadata->getHardwareGen();

coreStartEvents = aie::profile::getCoreEventSets(hwGen);
coreEndEvents = coreStartEvents;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: 'coreEndEvents' should be initialized in a member initializer of the constructor [cppcoreguidelines-prefer-member-initializer]

src/runtime_src/xdp/profile/plugin/aie_profile/client/aie_profile_npu3.cpp:49:

-     : AieProfileImpl(database, metadata)
+     : AieProfileImpl(database, metadata), coreEndEvents(coreStartEvents)
Suggested change
coreEndEvents = coreStartEvents;

coreEndEvents = coreStartEvents;

memoryStartEvents = aie::profile::getMemoryEventSets(hwGen);
memoryEndEvents = memoryStartEvents;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: 'memoryEndEvents' should be initialized in a member initializer of the constructor [cppcoreguidelines-prefer-member-initializer]

src/runtime_src/xdp/profile/plugin/aie_profile/client/aie_profile_npu3.cpp:49:

-     : AieProfileImpl(database, metadata)
+     : AieProfileImpl(database, metadata), memoryEndEvents(memoryStartEvents)
Suggested change
memoryEndEvents = memoryStartEvents;

memoryEndEvents = memoryStartEvents;

shimStartEvents = aie::profile::getInterfaceTileEventSets(hwGen);
shimEndEvents = shimStartEvents;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: 'shimEndEvents' should be initialized in a member initializer of the constructor [cppcoreguidelines-prefer-member-initializer]

src/runtime_src/xdp/profile/plugin/aie_profile/client/aie_profile_npu3.cpp:49:

-     : AieProfileImpl(database, metadata)
+     : AieProfileImpl(database, metadata), shimEndEvents(shimStartEvents)
Suggested change
shimEndEvents = shimStartEvents;

shimEndEvents = shimStartEvents;

memTileStartEvents = aie::profile::getMemoryTileEventSets(hwGen);
memTileEndEvents = memTileStartEvents;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: 'memTileEndEvents' should be initialized in a member initializer of the constructor [cppcoreguidelines-prefer-member-initializer]

src/runtime_src/xdp/profile/plugin/aie_profile/client/aie_profile_npu3.cpp:49:

-     : AieProfileImpl(database, metadata)
+     : AieProfileImpl(database, metadata), memTileEndEvents(memTileStartEvents)
Suggested change
memTileEndEvents = memTileStartEvents;

auto context = metadata->getHwContext();
uint32_t* output = nullptr;
std::map<uint32_t, size_t> activeUCsegmentMap;
activeUCsegmentMap[0] = 0x20000;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: 0x20000 is a magic number; consider replacing it with a named constant [cppcoreguidelines-avoid-magic-numbers]

    activeUCsegmentMap[0] = 0x20000;
                            ^

uint32_t* output = resultBO.map<uint32_t*>();

// Get timestamp in milliseconds
double timestamp = xrt_core::time_ns() / 1.0e6;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: narrowing conversion from 'unsigned long long' to 'double' [bugprone-narrowing-conversions]

    double timestamp = xrt_core::time_ns() / 1.0e6;
                       ^

double timestamp = xrt_core::time_ns() / 1.0e6;

//**************************TODO: Remove this after testing ***************************
for (u32 i = 0; i < op_profile_data.size() + 12 * 3; i++) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: 12 is a magic number; consider replacing it with a named constant [cppcoreguidelines-avoid-magic-numbers]

    for (u32 i = 0; i < op_profile_data.size() + 12 * 3; i++) {
                                                 ^

double timestamp = xrt_core::time_ns() / 1.0e6;

//**************************TODO: Remove this after testing ***************************
for (u32 i = 0; i < op_profile_data.size() + 12 * 3; i++) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: performing an implicit widening conversion to type 'size_type' (aka 'unsigned long') of a multiplication performed in type 'int' [bugprone-implicit-widening-of-multiplication-result]

    for (u32 i = 0; i < op_profile_data.size() + 12 * 3; i++) {
                                                 ^
Additional context

src/runtime_src/xdp/profile/plugin/aie_profile/client/aie_profile_npu3.cpp:389: make conversion explicit to silence this warning

    for (u32 i = 0; i < op_profile_data.size() + 12 * 3; i++) {
                                                 ^

src/runtime_src/xdp/profile/plugin/aie_profile/client/aie_profile_npu3.cpp:389: perform multiplication in a wider type

    for (u32 i = 0; i < op_profile_data.size() + 12 * 3; i++) {
                                                 ^

//**************************TODO: Remove this after testing ***************************
for (u32 i = 0; i < op_profile_data.size() + 12 * 3; i++) {
std::stringstream msg;
msg << "Counter address/values: " << output[2 * i] << " - " << output[2 * i + 1];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: result of multiplication in type 'u32' (aka 'unsigned int') is used as a pointer offset after an implicit widening conversion to type 'size_t' [bugprone-implicit-widening-of-multiplication-result]

      msg << "Counter address/values: " << output[2 * i] << " - " << output[2 * i + 1];
                                           ^
Additional context

src/runtime_src/xdp/profile/plugin/aie_profile/client/aie_profile_npu3.cpp:391: make conversion explicit to silence this warning

      msg << "Counter address/values: " << output[2 * i] << " - " << output[2 * i + 1];
                                                  ^

src/runtime_src/xdp/profile/plugin/aie_profile/client/aie_profile_npu3.cpp:391: perform multiplication in a wider type

      msg << "Counter address/values: " << output[2 * i] << " - " << output[2 * i + 1];
                                                  ^

for (u32 i = 0; i < op_profile_data.size(); i++) {
// Update counter value in outputValues and add to database
std::vector<uint64_t> values = outputValues[i];
values[5] = static_cast<uint64_t>(output[2 * i + 1]); // Write counter value
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: 5 is a magic number; consider replacing it with a named constant [cppcoreguidelines-avoid-magic-numbers]

      values[5] = static_cast<uint64_t>(output[2 * i + 1]); // Write counter value
             ^

Jain added 5 commits November 12, 2025 10:56
Signed-off-by: Jain <parthash@amd.com>
Signed-off-by: Jain <parthash@amd.com>
…aie_devel

Signed-off-by: Jain <parthash@amd.com>
Signed-off-by: Jain <parthash@amd.com>
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clang-tidy made some suggestions

auto hwGen = metadata->getHardwareGen();

coreStartEvents = aie::profile::getCoreEventSets(hwGen);
coreEndEvents = coreStartEvents;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: 'coreEndEvents' should be initialized in a member initializer of the constructor [cppcoreguidelines-prefer-member-initializer]

src/runtime_src/xdp/profile/plugin/aie_profile/client/aie_profile_npu3.cpp:50:

-     : AieProfileImpl(database, metadata, deviceID)
+     : AieProfileImpl(database, metadata, deviceID), coreEndEvents(coreStartEvents)
Suggested change
coreEndEvents = coreStartEvents;

coreEndEvents = coreStartEvents;

memoryStartEvents = aie::profile::getMemoryEventSets(hwGen);
memoryEndEvents = memoryStartEvents;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: 'memoryEndEvents' should be initialized in a member initializer of the constructor [cppcoreguidelines-prefer-member-initializer]

src/runtime_src/xdp/profile/plugin/aie_profile/client/aie_profile_npu3.cpp:50:

-     : AieProfileImpl(database, metadata, deviceID)
+     : AieProfileImpl(database, metadata, deviceID), memoryEndEvents(memoryStartEvents)
Suggested change
memoryEndEvents = memoryStartEvents;

memoryEndEvents = memoryStartEvents;

shimStartEvents = aie::profile::getInterfaceTileEventSets(hwGen);
shimEndEvents = shimStartEvents;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: 'shimEndEvents' should be initialized in a member initializer of the constructor [cppcoreguidelines-prefer-member-initializer]

src/runtime_src/xdp/profile/plugin/aie_profile/client/aie_profile_npu3.cpp:50:

-     : AieProfileImpl(database, metadata, deviceID)
+     : AieProfileImpl(database, metadata, deviceID), shimEndEvents(shimStartEvents)
Suggested change
shimEndEvents = shimStartEvents;

shimEndEvents = shimStartEvents;

memTileStartEvents = aie::profile::getMemoryTileEventSets(hwGen);
memTileEndEvents = memTileStartEvents;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: 'memTileEndEvents' should be initialized in a member initializer of the constructor [cppcoreguidelines-prefer-member-initializer]

src/runtime_src/xdp/profile/plugin/aie_profile/client/aie_profile_npu3.cpp:50:

-     : AieProfileImpl(database, metadata, deviceID)
+     : AieProfileImpl(database, metadata, deviceID), memTileEndEvents(memTileStartEvents)
Suggested change
memTileEndEvents = memTileStartEvents;

activeUCsegmentMap[0] = 0x20000;
try {
//resultBO = xrt_core::bo_int::create_debug_bo(context, 0x20000);
resultBO = xrt_core::bo_int::create_bo(context, 0x20000, xrt_core::bo_int::use_type::uc_debug);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: 0x20000 is a magic number; consider replacing it with a named constant [cppcoreguidelines-avoid-magic-numbers]

      resultBO = xrt_core::bo_int::create_bo(context, 0x20000, xrt_core::bo_int::use_type::uc_debug);
                                                      ^

1,
meta_config.mem_row_start,
meta_config.mem_num_rows,
meta_config.aie_tile_row_start,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: suggest braces around initialization of subobject [clang-diagnostic-missing-braces]

Suggested change
meta_config.aie_tile_row_start,
{meta_config.aie_tile_row_start,

src/runtime_src/xdp/profile/plugin/aie_profile/client/aie_profile_npu3.cpp:116:

-       {0} // PartProp
+       {0}} // PartProp

double timestamp = xrt_core::time_ns() / 1.0e6;

//**************************TODO: Remove this after testing ***************************
for (u32 i = 0; i < op_profile_data.size() + 12 * 3; i++) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: performing an implicit widening conversion to type 'size_type' (aka 'unsigned long') of a multiplication performed in type 'int' [bugprone-implicit-widening-of-multiplication-result]

    for (u32 i = 0; i < op_profile_data.size() + 12 * 3; i++) {
                                                 ^
Additional context

src/runtime_src/xdp/profile/plugin/aie_profile/client/aie_profile_npu3.cpp:390: make conversion explicit to silence this warning

    for (u32 i = 0; i < op_profile_data.size() + 12 * 3; i++) {
                                                 ^

src/runtime_src/xdp/profile/plugin/aie_profile/client/aie_profile_npu3.cpp:390: perform multiplication in a wider type

    for (u32 i = 0; i < op_profile_data.size() + 12 * 3; i++) {
                                                 ^

//**************************TODO: Remove this after testing ***************************
for (u32 i = 0; i < op_profile_data.size() + 12 * 3; i++) {
std::stringstream msg;
msg << "Counter address/values: " << output[2 * i] << " - " << output[2 * i + 1];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: result of multiplication in type 'u32' (aka 'unsigned int') is used as a pointer offset after an implicit widening conversion to type 'size_t' [bugprone-implicit-widening-of-multiplication-result]

      msg << "Counter address/values: " << output[2 * i] << " - " << output[2 * i + 1];
                                           ^
Additional context

src/runtime_src/xdp/profile/plugin/aie_profile/client/aie_profile_npu3.cpp:392: make conversion explicit to silence this warning

      msg << "Counter address/values: " << output[2 * i] << " - " << output[2 * i + 1];
                                                  ^

src/runtime_src/xdp/profile/plugin/aie_profile/client/aie_profile_npu3.cpp:392: perform multiplication in a wider type

      msg << "Counter address/values: " << output[2 * i] << " - " << output[2 * i + 1];
                                                  ^

AieProfile_NPU3Impl(VPDatabase* database, std::shared_ptr<AieProfileMetadata> metadata, uint64_t deviceID);
~AieProfile_NPU3Impl() = default;

void updateDevice();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: 'updateDevice' overrides a member function but is not marked 'override' [clang-diagnostic-inconsistent-missing-override]

    void updateDevice();
         ^
Additional context

src/runtime_src/xdp/profile/plugin/aie_profile/aie_profile_impl.h:40: overridden virtual function is here

    virtual void updateDevice() = 0;
                 ^

void poll(const uint64_t id) override;
void endPoll() override {}

void freeResources();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: 'freeResources' overrides a member function but is not marked 'override' [clang-diagnostic-inconsistent-missing-override]

    void freeResources();
         ^
Additional context

src/runtime_src/xdp/profile/plugin/aie_profile/aie_profile_impl.h:47: overridden virtual function is here

    virtual void freeResources() = 0;
                 ^

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants