[SWDEV-544298] AINIC implementation - #2798
Conversation
b3853f2 to
6b4db08
Compare
6b4db08 to
ac4181f
Compare
oliveiradan
left a comment
There was a problem hiding this comment.
Besides my individual comments, we still have build warnings to look at, as stated in the previous PR:
https://github.com/AMD-ROCm-Internal/rocm-systems/pull/441
8ea1846 to
86d6278
Compare
7bcfb73 to
ea94907
Compare
784fe1f to
da5c768
Compare
|
overriding therock windows ci failure known hiptests issue. legacy rocm ci is passing on amdsmi test |
| set(NIC_SOURCE_DIR ${PROJECT_ROOT}/src) | ||
| set(NIC_BUILD_DIR ${PROJECT_ROOT}/build) | ||
| set(NIC_INTERFACE_DIR ${PROJECT_ROOT}/interface) | ||
|
|
||
| set(NIC_DEFAULT_CXX_FLAGS "-Wall -Wextra -Werror -Wno-missing-field-initializers -Wmissing-declarations -Werror=conversion -Wshift-negative-value -fPIC") | ||
| set(NIC_DEFAULT_CXX_FLAGS "${NIC_DEFAULT_CXX_FLAGS} -Wl,-z,relro,-z,noexecstack,-z,noexecheap -Wl,--strip-debug -Wl,--strip-all") | ||
| set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} ${NIC_DEFAULT_CXX_FLAGS}") | ||
|
|
||
| file(GLOB CPP_SRCS "${NIC_SOURCE_DIR}/*.cpp") | ||
| add_library(amdsminic STATIC ${CPP_SRCS}) | ||
| set_target_properties(amdsminic PROPERTIES ARCHIVE_OUTPUT_DIRECTORY ${NIC_BUILD_DIR}) |
There was a problem hiding this comment.
This set(NIC_BUILD_DIR ${PROJECT_ROOT}/build) is not right - the build should not write into the source directory like this. See ROCm/TheRock#3500 (comment)
Quoting @stellaraccident :
Indeed. That is a major error. Revert or fix immediately.
RCCL grew one like this recently too.
ScottTodd
left a comment
There was a problem hiding this comment.
Why was this massive PR merged with such a short PR description? Given the release build issue reported in ROCm/TheRock#3500, let's revert.
There was a problem hiding this comment.
Why was this build log committed?
There was a problem hiding this comment.
drive-by: this file is huge... should it be split into multiple files? How are you reviewing changes to it?
Having issue reverting with github UI, will try to revert locally. |
This reverts commit 00e9b8d.
|
original PR: ROCm/amdsmi#71 |
* Revert "[SWDEV-544298] AINIC implementation (#2798)" This reverts commit 00e9b8d. * Revert "[ROCM-2942] Fix AMDSMI_STATUS_NO_DATA on NIC devices & tests not completing (#3289)" This reverts commit 3c85365. --------- Co-authored-by: Jenkins <jenkins-compute@amd.com> Co-authored-by: Maisam Arif <Maisam.Arif@amd.com>
Co-authored-by: Oosman Saeed <oossaeed@amd.com>
[ROCM-2942] Fix AMDSMI_STATUS_NO_DATA on NIC devices & tests not completing (#3289) Installation successful on attempt 1 Verifying installation: amdsmi.amdsmi_exception.AmdSmiLibraryException: Error code: 40 | AMDSMI_STATUS_NO_DATA - No data was found for given input Error: Process completed with exit code 1. Signed-off-by: Charis Poag <charis.poag@amd.com>
* Revert "[SWDEV-544298] AINIC implementation (#2798)" This reverts commit 00e9b8d. * Revert "[ROCM-2942] Fix AMDSMI_STATUS_NO_DATA on NIC devices & tests not completing (#3289)" This reverts commit 3c85365. --------- Co-authored-by: Jenkins <jenkins-compute@amd.com> Co-authored-by: Maisam Arif <Maisam.Arif@amd.com>
This change undoes the revert: bfe5604 Revert '[SWDEV-544298] AINIC implementation [#2798]' (#3418) 1f7eb27 SWDEV-544298_AINIC_implementation - Handle case where machine has no NIC and we are doing 'amd-smi static' (#3250) 00e9b8d [SWDEV-544298] AINIC implementation (#2798) The reason why it was reverted was that it was that the external CMake project, amdsmi_unified, was doing in-source build. We cannot change that CMakeLists.txt file to make it do out of source dir builds. This changes fixes that problem by copying the amdsmi_unified project into the build dir, then building it there, ensuring that even if it builds in its source dir, it is still in the parent project's build dir.
This change undoes the revert: bfe5604 Revert '[SWDEV-544298] AINIC implementation [#2798]' (#3418) 1f7eb27 SWDEV-544298_AINIC_implementation - Handle case where machine has no NIC and we are doing 'amd-smi static' (#3250) 00e9b8d [SWDEV-544298] AINIC implementation (#2798) The reason why it was reverted was that it was that the external CMake project, amdsmi_unified, was doing in-source build. We cannot change that CMakeLists.txt file to make it do out of source dir builds. This changes fixes that problem by copying the amdsmi_unified project into the build dir, then building it there, ensuring that even if it builds in its source dir, it is still in the parent project's build dir.
This change undoes the revert: bfe5604 Revert '[SWDEV-544298] AINIC implementation [#2798]' (#3418) 1f7eb27 SWDEV-544298_AINIC_implementation - Handle case where machine has no NIC and we are doing 'amd-smi static' (#3250) 00e9b8d [SWDEV-544298] AINIC implementation (#2798) The reason why it was reverted was that it was that the external CMake project, amdsmi_unified, was doing in-source build. We cannot change that CMakeLists.txt file to make it do out of source dir builds. This changes fixes that problem by copying the amdsmi_unified project into the build dir, then building it there, ensuring that even if it builds in its source dir, it is still in the parent project's build dir.
Motivation
Show AINIC information from amd-smi command line
Technical Details
Print out basic information about NIC, and detailed information
JIRA ID
SWDEV-544298
Test Plan
Added 2 additional tests
Test Result
Pass on device with AINIC
Submission Checklist
Commands:
amd-smi list --nic 0
amd-smi static --nic 0
TODO after this merge:
ESMI_LIB macro - does not compile if macro is OFF
headers for ROC-prof - replace with single header
refactor duplicated code that was commented upon in this PR