CMake: Use HIPIFY_OUTPUT_DIR when installing ROCm headers#491
CMake: Use HIPIFY_OUTPUT_DIR when installing ROCm headers#491daissi wants to merge 1 commit intopytorch:mainfrom
Conversation
When USE_ROCM is enabled, the install path for HIP headers was derived by replacing CMAKE_CURRENT_SOURCE_DIR in the header directory path. However, HIPified headers are generated into HIPIFY_OUTPUT_DIR, not CMAKE_CURRENT_SOURCE_DIR. As a result, the computed install path is incorrect. This change replaces CMAKE_CURRENT_SOURCE_DIR with HIPIFY_OUTPUT_DIR when rewriting header paths, ensuring ROCm headers are installed into the correct include/gloo/... location. This fixes incorrect include directory layout when building with ROCm. Signed-off-by: Dylan Aïssi <dylan.aissi@collabora.com>
|
Hi @daissi! 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 cla@meta.com. Thanks! |
GZGavinZhao
left a comment
GZGavinZhao
left a comment
There was a problem hiding this comment.
The change is reasonable to me. I'm curious though how the official ROCm PyTorch distribution was able to build/compile with the previous code. I assume it's because the official ROCm wheels have been built with gloo as a submodule so it doesn't have this problem...?
3 participants
When USE_ROCM is enabled, the install path for HIP headers was derived by replacing CMAKE_CURRENT_SOURCE_DIR in the header directory path.
However, HIPified headers are generated into HIPIFY_OUTPUT_DIR, not CMAKE_CURRENT_SOURCE_DIR. As a result, the computed install path is incorrect.
This change replaces CMAKE_CURRENT_SOURCE_DIR with HIPIFY_OUTPUT_DIR when rewriting header paths, ensuring ROCm headers are installed into the correct include/gloo/... location.
This fixes incorrect include directory layout when building with ROCm.
This patch is applied for the Debian packaging otherwise headers are installed in the wrong location.