Skip to content
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

Why are all the CMake projects added via ExternalProject_Add? #28

Open
jrmadsen opened this issue Jan 26, 2025 · 1 comment
Open

Why are all the CMake projects added via ExternalProject_Add? #28

jrmadsen opened this issue Jan 26, 2025 · 1 comment

Comments

@jrmadsen
Copy link

Hi, I’m curious about the decision to use ExternalProject_Add when all of our ROCm software packages use CMake.

In my experience, using CMake subprojects adds a significant amount of benefits. For example, the build DAG would be significantly improved (e.g. rocprofiler-sdk doesn’t actually need ROCr or HIP to be built, it just needs its headers). Another example would be for testing: a single ctest command could run the entire test suite. Furthermore, this project could provide CMake INTERFACE libraries which contain the build flags for different build configurations, e.g. if this project were to make a rocm::sanitizer target which contains the flags for enabling whichever address/leak/thread/undefined-behavior sanitizer was desired, all the subprojects would have to do is add $<TARGET_NAME_IF_EXISTS:rocm::sanitizer> to the target_link_libraries. I’ve got more examples of the benefits but hopefully these make my case.

@stellaraccident
Copy link
Contributor

Yeah, good question and one I was on the other side of in the beginning. There was a prototype that did what you suggest but it was a big mess for the following reasons:

  • I'm sure you know that "it's cmake" isn't the actual bar that must be met for projects to exist together in the same build. In this case, the rocm cmake projects are very much not designed to play together (to say nothing of adding llvm to the mix). Some experiments I did found exactly one case that wasn't basically at the level of "step 1 would be to rewrite the cmake files of all of the sub projects".
  • The full rocm SDK build has to be a super-project anyway because parts of the core and the compiler have to be built before the libraries and rest of the core (since the bootstrap parts build with the host toolchain and the "user" parts build with the AMD llvm toolchain+extras.
  • The individual rocm projects are full of layering violations that I assume come from a long history of having been built to source deps from each project's install prefix (with all kinds of context dependent fallback heuristics). These are really nasty to track down in a unified build, and I was pulling my hair out until I introduced true isolation in order to fuzz the situation and make the problems fail eagerly.

I ended up abandoning the unified build as a first step while trying to organize things so that a future refactor could make more of a unified build. It still wouldn't be just one invocation... Probably three or four like: compiler, core, math-libs, framwork-libs. I tried to set things up so that an eventual rework like that could be more of an incremental task.

This first phase is focused on containment of the sub libraries and making dependencies explicit. For that, we are staging each individual project separately, more like how an OS build does it. That is requiring significant archaeology and rework.

A full scrub of the sub project cmake is better done once the infra is in place to work at head and make more sweeping changes. As therock is laid out now, I would do that per top level directory at a future point.

Also, one nit: I didn't end up using ExternalProject but some custom utilities that do something similar. So it is still a sub-build but with more commonality, ergonomics, and some additional injected tooling to manage dependency resolution.

So in short: I partially agree on the appeal (at least within the "user space" parts). But I just couldn't find a practical way to get there in one step.

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

No branches or pull requests

2 participants