Repository navigation
Integrate debug-tools (amd-dbgapi, rocr-debug-agent and rocgdb) into TheRock - #2797
Conversation
973e6fc to
3c02f41
Compare
|
I spotted a couple issues with CI:
|
3c02f41 to
e6da3bd
Compare
|
I think I've found the rocr-debug-agent artifact issue. It was some leftover erroneous code where I had included rocr-debug-agent artifacts in the else leg here: I've pushed a new series including all of the mpfr/expat/ncurses changes from #2796. |
|
Looks like the Windows failures are infra-related so far. The Linux ones are due to a lack of libpython. |
e6da3bd to
474c1e9
Compare
|
Updated the series. After the merge of #2796, we're left with 3 commits that include amd-dbgapi, rocr-debug-agent and rocgdb. I have another patch touching dockerfiles/build_manylinux_x86_64.Dockerfile, which should hopefully address the python-dev system dependency. But I'd like to confirm that this is the right place to do it. The rocgdb cmake wrapper was updated to include the rocgdb python selector script and some re-shuffling of code. |
|
A pushed a couple of patches that I think get this to landable. I will do a separate PR for needed build container changes. Then once we land this, it will build rocgdb without python support. Once the build container is updated, we will add a flag in CI to set THEROCK_SHARED_PYTHON_EXECUTABLES correctly and then we should get a full rocgdb with embedded python. I staged it this way because it lets us opt in to the full treatment and if individual devs don't have all of the fiddly python details correct, they will just get a rocgdb without python until they fix it. |
stellaraccident
left a comment
There was a problem hiding this comment.
Needs updates to the build container before it can land.
* texinfo-tex (discovered missing while working) * Shared builds of python 3.10-14 so that rocgdb can link against versions suitable for embedding python. Note that we build these python variants from source sufficient for build time use: they do not have all features. Adds an estimated ~250MiB to the container size and is unavoidable. Container build on a regular workstation doesn't take much more time. On the default GHA runner, it adds about 10m to the build time (12m total), which is not consequential. Will unblock #2797 once we roll the hashes to this new version.
c5552e2 to
2a9e50a
Compare
|
Thanks, the changes make the wrapper much cleaner. I've added another commit on top to fixup the testsuite installation structure. Otherwise these changes look good to me. I took the liberty of adding/structuring the commit messages for your 2 commits. I gave this a try locally and I get rocgdb built without python support as expected. Next step is probably making sure we can do the proper builds with the various versions of libpython when they become available. |
2a9e50a to
28d5b97
Compare
* Motivation Add the debug-tools feature that will serve as the home to all debug tools projects we have: ROCdbgapi, ROCgdb and ROCR-debug-agent. This commit adds the feature along with ROCdbgapi. Please note this is restricted to Linux builds for now. Windows builds will come at a later stage. * Technical Details This commit creates the minimal set of changes required to support the ROCm debug tools in TheRock build platform. It adds the following: - New debug-tools features/subdirectory (updated main CMakeLists.txt) - Adds ROCdbgapi as a submodule using github's amd-staging branch. - Adds the main debug-tools/CMakeLists.txt file with instructions on how to build the debug tools. At the moment this is just ROCdbgapi, named amd-dbgapi. - Updates build_tools/fetch_sources.py - Updates the documentation. - Adds ROCdbgapi artifacts. NOTE 1: ROCdbgapi will be added as a submodule of TheRock only temporarily until a decision is made on the topic. It may be the case we will want to migrate it to rocm-systems. But adding it as a submodule now allows us to integrate the component into TheRock without having to wait for all the rocm-systems migration steps to take place. NOTE 2: To make integration and testing faster, we have a local amd-dbgapi.cmake file that gets copied into the source directory before builds. Later on these cmake changes will be integrated into ROCdbgapi and the local file will be removed. * Test Plan - Validate that ROCdbgapi builds and pulls its libbacktrace dependency. - Validate that ROCdbgapi's shared object exists and matches the name. * Test Result All tests passed.
* Motivation Add rocr-debug-agent to the debug tools feature, to be built alongside the ROCdbgapi. Please note this is restricted to Linux builds for now. Windows builds will come at a later stage. * Technical Details - Add rocr-debug-agent as a submodule using github's amd-staging branch. - Adjust debug-tools/CMakeLists.txt to handle the new rocr-debug-agent component. - Add rocr-debug-agent artifacts. - Add separate testing component rocr-debug-agent-tests. - Add rocr-debug-agent-tests artifacts. NOTE 1: Similarly to ROCdbgapi, rocr-debug-agent is integrated as a module only temporarily so we can test the integration. It will be migrated to rocm-systems later on. When that happens, we can update the build recipes. NOTE 2: As with ROCdbgapi, we have a local cmake build file that gets copied prior to building the component. This is to speed up integration and testing and the local cmake file will be removed in the future and its modification pushed to rocr-debug-agent. * Test Plan - Validate that rocr-debug-agent builds and pulls its amd-dbgapi dependency. - Validate that rocr-debug-agent shared object and test binary exists and matches the required names. - Validate that rocr-debug-agent-tests builds correctly for the specified gfx architectures. - Validate that rocr-debug-agent-tests contents exist. * Test Result All tests passed.
* Motivation Add rocgdb to the debug tools feature, to be built alongside the ROCdbgapi. Please note this is restricted to Linux builds for now. Windows builds will come at a later stage. * Technical Details - Adjust debug-tools/CMakeLists.txt to handle the new rocgdb component. - Add new rocgdb submodule that will populate debug-tools/rocgdb with amd-staging branch of https://github.com/ROCm/ROCgdb. - New rocgdb.cmake wrapper to build gdb since its build system is autotools-based. * Test Plan - Validate that rocgdb builds and pulls its amd-dbgapi dependency. - Validate that rocgdb installs its files in their right places. - Validate that rocgdb is functional when built within TheRock. * Test Result All tests passed. Requires bison, flex, gmp, mpfr, expat, texinfo, ncurses, ncurses-dev, python-dev, liblzma, libz, zstd and librocm-dbgapi.
Move rocgdb checkout to debug-tools/rocgdb/source so we can hold the CMakeLists.txt wrapper under debug-tools/rocgdb. This way the cmake file gets picked up without having to copy it to debug-tools/rocgdb/source.
[debug-tools/rocgdb] * Pass through the C/CXX compiler that CMake detects. * Moves rpath patching to the install phase to avoid some possible races and align with how it is done elsewhere. * Remove the execution of the testing suite from within the build. We still install the test suite and should run it post build. But not great to fail the build in the middle due to test issues. * Added new THEROCK_SHARED_PYTHON_EXECUTABLES top level config variable to explicitly have the shared library python's we want to build with for embedded python cases (like rocgdb). I made the heuristic tri-state: If given, then all of them must have been built shared (or error). If not given but the default Python3_EXECUTABLE is suitable, that will be used. If not suitable, a warning will be issued that rocgdb is being build without python support. This balances the CI/CD case where we want to fail eagerly if the configuration is not valid with the developer case where we would like them to be able to make progress even if their environment is missing these components. * Refactored the rocgdb cmake to support no-python and embedded python modes. Also updated the wrapper to fallback to "pynone", which will hit it built on a machine without a suitable Python for embedded use. * Misc CMake quality of life improvements made. [cmake] * Added a helper therock_detect_shared_python_executables for the above and tested it on Alma8 using a each of the states.
…iles. Tweak the testsuite installation some more so the "source" directory is hardcoded on the instructions as opposed to the filenames. This makes it so we install everything under tests/rocgdb/* as opposed to installing some files under tests/rocgdb/source/*.
28d5b97 to
4fc727e
Compare
stellaraccident
left a comment
There was a problem hiding this comment.
If CI passes, LGTM to land.
|
Looks like the artifact installation is grumpy for some reason. Investigating... |
|
Something off with the artifact manifest for rocgdb. In particular the bin/rocnm file. |
The rocgdb artifact contains binutils tools that use hardlinks (e.g., rocnm -> nm). The ArtifactPopulator was failing with "Unhandled tar member" when extracting these archives. Add handling for tar hardlinks using member.islnk() and os.link(). The fix computes the target file's destination path by matching against manifest relpaths, supporting both flatten and non-flatten modes. Includes a unit test that verifies hardlinks are preserved through the archive/flatten cycle (skipped on Windows). 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
|
Doing some more extensive testing with all of the available python-shared entries, I ran into a rocgdb configure issue that I'm investigating. Let's wait until I have ironed it out. Context: I think we're missing passing the libpython directory somewhere, for the linker. I'm working on a fix. |
|
Windows/miopen crash is not related. Filed an issue separately/above. Looks like we are past the point of prior failure and this is going to pass this time. Will land when the rest of the test jobs finish. |
I think we can/should land as is as a checkpoint. That's why I set it up so that it isn't doing multi Python yet. We can treat that as an enhancement and put this big PR behind us. |
Up to you though. |
Agreed. Merged now. |
| parser.add_argument( | ||
| "--debug-tools", | ||
| nargs="+", | ||
| type=str, | ||
| default=[ | ||
| "amd-dbgapi", | ||
| "rocr-debug-agent", | ||
| "rocgdb", | ||
| ], | ||
| ) |
There was a problem hiding this comment.
Are these projects Linux-only?
Should we exclude these submodule fetches on Windows, using the pattern from above?
parser.add_argument(
"--rocm-media-projects",
nargs="+",
type=str,
default=(
[]
if is_windows()
else [
# Linux only projects.
"amd-mesa",
]
),
)I noticed that the rocgdb repository is 500MB which is fairly large, not as large as LLVM (2.3GB), rocm-systems (1GB), or rocm-libraries (9.8GB) though.
There was a problem hiding this comment.
Linux-only for now. And we probably should exclude source fetches on Windows, if that is a possibility. Personally I wasn't aware we could do it.
Would this save some downloading time if someone is on a Windows host?
There was a problem hiding this comment.
Yes, and it is worth optimizing the list of fetched sources since the fetch sources step is overhead on every workflow run.
See recent logs like https://github.com/ROCm/TheRock/actions/runs/20965480079/job/60254608548#step:11:62 for example, where the "fetch sources" step took 7m30s on a Windows build VM.
(Separate from this we plan on seeding the build VMs with a git checkout so they aren't always starting from scratch)
There was a problem hiding this comment.
Alright. Thanks for clarifying it. I'll put a PR together for the debug-tools.
There was a problem hiding this comment.
Oh, ROCgdb is supported on Windows? https://rocm.docs.amd.com/projects/install-on-windows/en/latest/how-to/debugger-windows.html
We should clarify the docs at https://github.com/ROCm/TheRock/blob/main/docs/development/windows_support.md#supported-subprojects to indicate that support is not yet added to TheRock, but it is available.
Integrate 3 debugging tools into TheRock under the debug-tools logical group:
Building of those tools is controlled by the -DTHEROCK_ENABLE_DEBUG_TOOLS switch. And the individual tools can also be included/excluded.
At the moment rocgdb builds without python support (rocgdb-pynone).
This commit also enables TheRock to detect Python shared library support (libpython) and adds hardlink support to the artifact populator.
Co-authored-by: Stella Laurenzo stellaraccident@gmail.com
Co-authored-by: Claude noreply@anthropic.com