-
Notifications
You must be signed in to change notification settings - Fork 516
Add common nop.elf submission utility for VE2 AIE profile and trace #9524
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
|
clang-tidy review says "All clean, LGTM! 👍" |
9b42c68 to
f27f149
Compare
|
clang-tidy review says "All clean, LGTM! 👍" |
- Create aie_nop_util.h/cpp with submitNopElf() function in aie_base - Update aie_profile.cpp (VE2) to call submitNopElf before setMetricsSettings - Move nop.elf submission for AIE trace to initReadTrace() in aie_trace_offload_ve2.cpp (before BD configuration instead of updateDevice) - Add xrt_core to link libraries in CMakeLists.txt for VE2 builds Signed-off-by: Jyotheeswar Ganne <[email protected]>
f27f149 to
b72a712
Compare
|
clang-tidy review says "All clean, LGTM! 👍" |
| add_library(xdp_aie_profile_plugin_xdna SHARED ${AIE_PROFILE_PLUGIN_FILES} ${AIE_PROFILE_IMPL_FILES} ${AIE_PROFILE_UTIL_FILES} ${AIE_PROFILE_CONFIG_FILES}) | ||
| add_dependencies(xdp_aie_profile_plugin_xdna xdp_core xrt_coreutil) | ||
| target_link_libraries(xdp_aie_profile_plugin_xdna PRIVATE xdp_core xrt_coreutil xaiengine) | ||
| target_link_libraries(xdp_aie_profile_plugin_xdna PRIVATE xdp_core xrt_coreutil xrt_core xaiengine) |
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.
Why do we need to link to xrt_core here?
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.
we need it for xrt::elf and xrt::module used in submitNopElf().
| add_library(xdp_aie_trace_plugin_xdna SHARED ${AIE_TRACE_PLUGIN_FILES} ${AIE_TRACE_COMPONENT_FILES} ${AIE_TRACE_UTIL_FILES} ${AIE_TRACE_CONFIG_FILES}) | ||
| add_dependencies(xdp_aie_trace_plugin_xdna xdp_core xrt_coreutil) | ||
| target_link_libraries(xdp_aie_trace_plugin_xdna PRIVATE xdp_core xrt_coreutil xaiengine) | ||
| target_link_libraries(xdp_aie_trace_plugin_xdna PRIVATE xdp_core xrt_coreutil xrt_core xaiengine) |
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.
Why do we need to link to xrt_core here?
| return; | ||
|
|
||
| // Submit nop.elf before configuring profile | ||
| if (!aie::submitNopElf(metadata->getHandle())) |
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.
Please add a warning message when submission of nop elf and hence profile configuration does not go through.
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.
Added.
| bool AIETraceOffload::initReadTrace() | ||
| { | ||
| // Submit nop.elf to initialize AIE array before BD configuration | ||
| if (!aie::submitNopElf(deviceHandle)) |
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.
Please add a warning message when submission of nop elf and hence trace configuration does not go through.
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.
Added.
Add warning messages in aie_profile and aie_trace_offload to inform user when nop.elf submission fails and configuration will not proceed. Signed-off-by: Jyotheeswar Ganne <[email protected]>
|
clang-tidy review says "All clean, LGTM! 👍" |
xrt_core is not needed for the nop.elf submission functionality. Signed-off-by: Jyotheeswar Ganne <[email protected]>
|
clang-tidy review says "All clean, LGTM! 👍" |
|
retest this please |
Problem solved by the commit
Without nop.elf submission, the AIE array is clock gated and both AIE profile and AIE trace are unable to configure the AIE array. By submitting nop.elf first, the AIE array is initialized and tiles are no longer gated, allowing profile/trace configuration to proceed.
This commit adds a common submitNopElf() utility function to avoid code duplication between aie_profile and aie_trace for VE2.
Bug / issue (if any) fixed, which PR introduced the bug, how it was discovered
N/A - New feature for VE2 AIE profile and trace support.
How problem was solved, alternative solutions (if any) and why they were rejected
Created a common utility function submitNopElf() in aie_base/aie_nop_util.cpp that loads and executes nop.elf before setMetricsSettings(). Both VE2 aie_profile.cpp and aie_trace.cpp call this shared function. Added xrt_core to link libraries for VE2 plugins.
Risks (if any) associated the changes in the commit
None.
What has been tested and how, request additional testing if necessary
Tested on a VE2 design.
Documentation impact (if any)