-
Notifications
You must be signed in to change notification settings - Fork 516
[XDP] Initial NPU3 support #9130
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
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>
There was a problem hiding this 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, |
There was a problem hiding this comment.
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(), |
There was a problem hiding this comment.
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>
fd0a41a to
d475a92
Compare
stsoe
left a comment
There was a problem hiding this 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.
d475a92 to
fd0a41a
Compare
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>
…t_gen_aie_devel
…t_gen_aie_devel
Signed-off-by: Jain <parthash@amd.com>
…t_gen_aie_devel
There was a problem hiding this 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; |
There was a problem hiding this comment.
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)| coreEndEvents = coreStartEvents; | |
| coreEndEvents = coreStartEvents; | ||
|
|
||
| memoryStartEvents = aie::profile::getMemoryEventSets(hwGen); | ||
| memoryEndEvents = memoryStartEvents; |
There was a problem hiding this comment.
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)| memoryEndEvents = memoryStartEvents; | |
| memoryEndEvents = memoryStartEvents; | ||
|
|
||
| shimStartEvents = aie::profile::getInterfaceTileEventSets(hwGen); | ||
| shimEndEvents = shimStartEvents; |
There was a problem hiding this comment.
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)| shimEndEvents = shimStartEvents; | |
| shimEndEvents = shimStartEvents; | ||
|
|
||
| memTileStartEvents = aie::profile::getMemoryTileEventSets(hwGen); | ||
| memTileEndEvents = memTileStartEvents; |
There was a problem hiding this comment.
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)| memTileEndEvents = memTileStartEvents; | |
| auto context = metadata->getHwContext(); | ||
| uint32_t* output = nullptr; | ||
| std::map<uint32_t, size_t> activeUCsegmentMap; | ||
| activeUCsegmentMap[0] = 0x20000; |
There was a problem hiding this comment.
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; |
There was a problem hiding this comment.
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++) { |
There was a problem hiding this comment.
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++) { |
There was a problem hiding this comment.
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]; |
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
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
^Signed-off-by: Jain <parthash@amd.com>
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>
There was a problem hiding this 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; |
There was a problem hiding this comment.
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)| coreEndEvents = coreStartEvents; | |
| coreEndEvents = coreStartEvents; | ||
|
|
||
| memoryStartEvents = aie::profile::getMemoryEventSets(hwGen); | ||
| memoryEndEvents = memoryStartEvents; |
There was a problem hiding this comment.
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)| memoryEndEvents = memoryStartEvents; | |
| memoryEndEvents = memoryStartEvents; | ||
|
|
||
| shimStartEvents = aie::profile::getInterfaceTileEventSets(hwGen); | ||
| shimEndEvents = shimStartEvents; |
There was a problem hiding this comment.
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)| shimEndEvents = shimStartEvents; | |
| shimEndEvents = shimStartEvents; | ||
|
|
||
| memTileStartEvents = aie::profile::getMemoryTileEventSets(hwGen); | ||
| memTileEndEvents = memTileStartEvents; |
There was a problem hiding this comment.
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)| 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); |
There was a problem hiding this comment.
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, |
There was a problem hiding this comment.
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]
| 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++) { |
There was a problem hiding this comment.
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]; |
There was a problem hiding this comment.
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(); |
There was a problem hiding this comment.
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(); |
There was a problem hiding this comment.
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;
^
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.