Enable ROCm/MI325X build for RCCLX and TorchComms #74
Conversation
|
Hi @Xinyu-Kang! Thank you for your pull request and welcome to our community. Action RequiredIn order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you. ProcessIn order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA. Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with If you have received this in error or have any questions, please contact us at [email protected]. Thanks! |
| function ensure_static_symlink() { | ||
| local base="$1" | ||
| local so="${CONDA_PREFIX}/lib/lib${base}.so" | ||
| local ar="${CONDA_PREFIX}/lib/lib${base}.a" |
There was a problem hiding this comment.
This seems a bit hacky as we're altering the conda environment. Why do we need static libraries in the first place? Can't we link to the shared libraries?
There was a problem hiding this comment.
Yea.. Currently rcclx CMake hard-codes the static archives (.a files), but the ROCm conda env I’m testing on only has the .so variants. It makes sense to just build against shared libraries, but I'm not sure if it will touch something else on other machines.
If you’d prefer, I can instead patch the rcclx CMake to:
- use the existing static libraries when they’re available, and
- fall back to the shared libraries when only .so is present,
and then drop ensure_static_symlink entirely so we don’t touch the conda environment.
Does that sound reasonable?
| #include <cuda.h> | ||
| #include <cuda_bf16.h> | ||
| #include <cuda_fp16.h> | ||
| using bf16 = __nv_bfloat16; |
There was a problem hiding this comment.
Why do we redefine these here if we already have them in comms/common/algorithms/CollCommon.cuh which gets included below?
|
Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Meta Open Source project. Thanks! |
meta-cla
bot
added
the
CLA Signed
2 participants
Description
rcclxandtorchcommswere not building on ROCm (MI325X). This PR makes the ROCm build work while keeping the CUDA path unchanged.Changes
CollCommon.cuhand the DDA kernels (all_reduce / all_gather / all_to_all / reduce_scatter).bf16,bf162, and__nv_bfloat16*shims on HIP) so the same kernels compile under both CUDA and HIP without hipify collisions.build_rcclx.shrobust on ROCmLD_LIBRARY_PATH/LIBRARY_PATHfrom the active conda env..so→.awhen only shared libs exist in the env (e.g.libssl,libcrypto,libevent,libsodium,libboost_context).THIRD_PARTY_ROOT(default${BASE_DIR}/.third-party-cache) instead of/tmp/third-partyto avoid permission errors, and add.forge_builtstamp files plusFORCE_THIRD_PARTY_REBUILDto control reuse.torchcommsbuild on ROCm by gating CUDA-only transportcomms/torchcomms/transport/CMakeLists.txt, treat CUDA as optional and fail early with a clear message if transport is enabled but CUDA is not found.setup.py, detect ROCm viatorch.version.hipand defaultUSE_TRANSPORTtoFalseon ROCm (still opt-in viaUSE_TRANSPORT=1if someone really wants to try). CUDA builds keep the existing default.