Fix hardcoded ROCm paths in `Caffe2Targets.cmake` by naromero77amd · Pull Request #136283 · pytorch/pytorch

naromero77amd · 2024-09-18T19:14:57Z

Use CMake imported targets more consistently to eliminate hardcode paths.

Here is the new relevant sections of Caffe2Targets.cmake:

set_target_properties(c10_hip PROPERTIES
  INTERFACE_INCLUDE_DIRECTORIES "${_IMPORT_PREFIX}/include"
  INTERFACE_LINK_LIBRARIES "c10;hip::amdhip64"
)

set_target_properties(torch_hip PROPERTIES
  INTERFACE_COMPILE_DEFINITIONS "USE_C10D_NCCL"
  INTERFACE_COMPILE_OPTIONS "-fPIC;-D__HIP_PLATFORM_AMD__=1;-DCUDA_HAS_FP16=1;-DUSE_ROCM;-D__HIP_NO_HALF_OPERATORS__=1;-D__HIP_NO_HALF_CONVERSIONS__=1;-DTORCH_HIP_VERSION=602;-Wno-shift-count-negative;-Wno-shift-count-overflow;-Wno-duplicate-decl-specifier;-DCAFFE2_USE_MIOPEN;-DTHRUST_DEVICE_SYSTEM=THRUST_DEVICE_SYSTEM_HIP;-std=c++17;-DHIPBLAS_V2;-DHIP_NEW_TYPE_ENUMS"
  INTERFACE_INCLUDE_DIRECTORIES "${_IMPORT_PREFIX}/include"
  INTERFACE_LINK_LIBRARIES "c10_hip;torch_cpu_library;hip::amdhip64;MIOpen;hiprtc::hiprtc;roc::hipblaslt;roc::hipblas;hip::hipfft;hip::hiprand;roc::hipsparse;roc::hipsolver"
)

HIPCUB dependency was not actually used; which is why it is removed here as the imported target had undesirable side effects.

cc @jeffdaily @sunway513 @jithunnair-amd @pruthvistony @ROCmSupport @dllehr-amd @jataylo @hongxiayang

pytorch-bot · 2024-09-18T19:15:01Z

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/136283

📄 Preview Python docs built from this PR
📄 Preview C++ docs built from this PR
❓ Need help or want to give feedback on the CI? Visit the bot commands wiki or our office hours

Note: Links to docs will display an error until the docs builds have been completed.

❌ 4 New Failures, 6 Unrelated Failures

As of commit e69fbc5 with merge base cf31724 ():

NEW FAILURES - The following jobs have failed:

linux-aarch64-binary-manywheel / manywheel-py3_10-cuda-aarch64-build / build (gh)
ninja: build stopped: subcommand failed
linux-aarch64-binary-manywheel / manywheel-py3_11-cuda-aarch64-build / build (gh)
ninja: build stopped: subcommand failed
linux-aarch64-binary-manywheel / manywheel-py3_12-cuda-aarch64-build / build (gh)
ninja: build stopped: subcommand failed
linux-aarch64-binary-manywheel / manywheel-py3_9-cuda-aarch64-build / build (gh)
ninja: build stopped: subcommand failed

FLAKY - The following jobs failed but were likely due to flakiness present on trunk:

linux-binary-manywheel / manywheel-py3_10-rocm6_2-upload / upload (gh) (matched linux rule in flaky-rules.json)
Error response from daemon: unauthorized: authentication required
linux-binary-manywheel-split / manywheel-py3_10-cpu-test / test (gh) (detected as infra flaky with no log or failing log classifier)
linux-binary-manywheel-split / manywheel-py3_11-cpu-test / test (gh) (detected as infra flaky with no log or failing log classifier)
linux-binary-manywheel-split / manywheel-py3_12-cpu-test / test (gh) (detected as infra flaky with no log or failing log classifier)
linux-binary-manywheel-split / manywheel-py3_13-cpu-test / test (gh) (detected as infra flaky with no log or failing log classifier)
linux-binary-manywheel-split / manywheel-py3_9-cpu-test / test (gh) (detected as infra flaky with no log or failing log classifier)

This comment was automatically generated by Dr. CI and updates every 15 minutes.

naromero77amd · 2024-09-18T19:35:09Z

@pytorchbot label "topic: not user facing"

naromero77amd · 2024-09-18T19:41:44Z

@pytorchbot label "ciflow/rocm"

pytorch-bot · 2024-09-18T19:41:52Z

Can't add following labels to PR: ciflow/rocm. Please ping one of the reviewers for help.

pytorch-bot · 2024-09-18T19:43:00Z

Please seek CI approval before scheduling CIFlow labels

jeffdaily

sure why not. if CI is green and this fixes the issue, good to go.

naromero77amd · 2024-09-20T22:24:37Z

@pytorchbot rebase

pytorchmergebot · 2024-09-20T22:26:09Z

@pytorchbot started a rebase job onto refs/remotes/origin/viable/strict. Check the current status here

…ROCM paths.

pytorchmergebot · 2024-09-20T22:26:13Z

Successfully rebased fix_caffe2_hardcoded_rocm onto refs/remotes/origin/viable/strict, please pull locally before adding more changes (for example, via git checkout fix_caffe2_hardcoded_rocm && git pull --rebase)

c10/hip/CMakeLists.txt

cmake/Dependencies.cmake

atalman

lgtm

naromero77amd · 2024-09-24T21:23:04Z

I can confirm that this PR has resolved deepmodeling/deepmd-kit#4008

jithunnair-amd · 2024-09-26T00:32:54Z

@pytorchbot merge -f "CI failures unrelated; changes only impact PyTorch builds and confirmed to fix related issue"

pytorchmergebot · 2024-09-26T00:34:34Z

Merge started

Your change will be merged immediately since you used the force (-f) flag, bypassing any CI checks (ETA: 1-5 minutes). Please use -f as last resort and instead consider -i/--ignore-current to continue the merge ignoring current failures. This will allow currently pending tests to finish and report signal before the merge.

Learn more about merging in the wiki.

Questions? Feedback? Please reach out to the PyTorch DevX Team

Advanced Debugging

Check the merge workflow status
here

jithunnair-amd · 2024-09-26T00:36:39Z

@pytorchbot cherry-pick --onto release/2.5 -c critical

Fixes #131701 Use CMake imported targets more consistently to eliminate hardcode paths. Here is the new relevant sections of Caffe2Targets.cmake: ``` set_target_properties(c10_hip PROPERTIES INTERFACE_INCLUDE_DIRECTORIES "${_IMPORT_PREFIX}/include" INTERFACE_LINK_LIBRARIES "c10;hip::amdhip64" ) ``` ``` set_target_properties(torch_hip PROPERTIES INTERFACE_COMPILE_DEFINITIONS "USE_C10D_NCCL" INTERFACE_COMPILE_OPTIONS "-fPIC;-D__HIP_PLATFORM_AMD__=1;-DCUDA_HAS_FP16=1;-DUSE_ROCM;-D__HIP_NO_HALF_OPERATORS__=1;-D__HIP_NO_HALF_CONVERSIONS__=1;-DTORCH_HIP_VERSION=602;-Wno-shift-count-negative;-Wno-shift-count-overflow;-Wno-duplicate-decl-specifier;-DCAFFE2_USE_MIOPEN;-DTHRUST_DEVICE_SYSTEM=THRUST_DEVICE_SYSTEM_HIP;-std=c++17;-DHIPBLAS_V2;-DHIP_NEW_TYPE_ENUMS" INTERFACE_INCLUDE_DIRECTORIES "${_IMPORT_PREFIX}/include" INTERFACE_LINK_LIBRARIES "c10_hip;torch_cpu_library;hip::amdhip64;MIOpen;hiprtc::hiprtc;roc::hipblaslt;roc::hipblas;hip::hipfft;hip::hiprand;roc::hipsparse;roc::hipsolver" ) ``` HIPCUB dependency was not actually used; which is why it is removed here as the imported target had undesirable side effects. Pull Request resolved: #136283 Approved by: https://github.com/jeffdaily, https://github.com/Skylion007, https://github.com/jithunnair-amd, https://github.com/atalman (cherry picked from commit e8f1dd6)

pytorchbot · 2024-09-26T00:40:39Z

Cherry picking #136283

The cherry pick PR is at #136700 and it is recommended to link a critical cherry pick PR with an issue.

Details for Dev Infra team

Raised by workflow job

Fix hardcoded ROCm paths in `Caffe2Targets.cmake` (#136283) Fixes #131701 Use CMake imported targets more consistently to eliminate hardcode paths. Here is the new relevant sections of Caffe2Targets.cmake: ``` set_target_properties(c10_hip PROPERTIES INTERFACE_INCLUDE_DIRECTORIES "${_IMPORT_PREFIX}/include" INTERFACE_LINK_LIBRARIES "c10;hip::amdhip64" ) ``` ``` set_target_properties(torch_hip PROPERTIES INTERFACE_COMPILE_DEFINITIONS "USE_C10D_NCCL" INTERFACE_COMPILE_OPTIONS "-fPIC;-D__HIP_PLATFORM_AMD__=1;-DCUDA_HAS_FP16=1;-DUSE_ROCM;-D__HIP_NO_HALF_OPERATORS__=1;-D__HIP_NO_HALF_CONVERSIONS__=1;-DTORCH_HIP_VERSION=602;-Wno-shift-count-negative;-Wno-shift-count-overflow;-Wno-duplicate-decl-specifier;-DCAFFE2_USE_MIOPEN;-DTHRUST_DEVICE_SYSTEM=THRUST_DEVICE_SYSTEM_HIP;-std=c++17;-DHIPBLAS_V2;-DHIP_NEW_TYPE_ENUMS" INTERFACE_INCLUDE_DIRECTORIES "${_IMPORT_PREFIX}/include" INTERFACE_LINK_LIBRARIES "c10_hip;torch_cpu_library;hip::amdhip64;MIOpen;hiprtc::hiprtc;roc::hipblaslt;roc::hipblas;hip::hipfft;hip::hiprand;roc::hipsparse;roc::hipsolver" ) ``` HIPCUB dependency was not actually used; which is why it is removed here as the imported target had undesirable side effects. Pull Request resolved: #136283 Approved by: https://github.com/jeffdaily, https://github.com/Skylion007, https://github.com/jithunnair-amd, https://github.com/atalman (cherry picked from commit e8f1dd6) Co-authored-by: Nichols A. Romero <nick.romero@amd.com>

naromero77amd requested review from jeffdaily and jithunnair-amd as code owners September 18, 2024 19:14

pytorch-bot bot added the module: rocm AMD GPU support for Pytorch label Sep 18, 2024

pytorchbot added the open source label Sep 18, 2024

naromero77amd mentioned this pull request Sep 18, 2024

/opt/rocm/lib/libamdhip64.so is hardcoded in Caffe2Targets.cmake in ROCm wheels #131701

Closed

pytorch-bot bot added the topic: not user facing topic category label Sep 18, 2024

hongxiayang added the ciflow/rocm Trigger "default" config CI on ROCm label Sep 18, 2024

pytorch-bot bot removed the ciflow/rocm Trigger "default" config CI on ROCm label Sep 18, 2024

bdhirsh added the triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module label Sep 19, 2024

jeffdaily approved these changes Sep 20, 2024

View reviewed changes

naromero77amd added 3 commits September 20, 2024 22:26

Add hiprtc package from ROCM directory.

c3efe64

For c10/hip, use the imported HIP target to avoid hard-coded ROCM paths.

0d13564

For Caffe2, use the imported HIP and ROC targets to avoid hard-coded …

e69fbc5

…ROCM paths.

pytorchmergebot force-pushed the fix_caffe2_hardcoded_rocm branch from 9ffda77 to e69fbc5 Compare September 20, 2024 22:26

Skylion007 approved these changes Sep 24, 2024

View reviewed changes

jithunnair-amd requested changes Sep 24, 2024

View reviewed changes

c10/hip/CMakeLists.txt Show resolved Hide resolved

cmake/Dependencies.cmake Show resolved Hide resolved

pruthvistony added the rocm This tag is for PRs from ROCm team label Sep 24, 2024

atalman added the ciflow/binaries_wheel Trigger binary build and upload jobs for wheel on the PR label Sep 24, 2024

atalman approved these changes Sep 24, 2024

View reviewed changes

jithunnair-amd added the ciflow/rocm Trigger "default" config CI on ROCm label Sep 24, 2024

mattaezell mentioned this pull request Sep 25, 2024

[Installation]: Issues with installing vLLM on ROCM without sudo access vllm-project/vllm#8042

Closed

1 task

jithunnair-amd approved these changes Sep 26, 2024

View reviewed changes

pytorchmergebot added the merging label Sep 26, 2024

pytorchmergebot added the Merged label Sep 26, 2024

pytorchmergebot closed this in e8f1dd6 Sep 26, 2024

pytorchmergebot removed the merging label Sep 26, 2024

jithunnair-amd mentioned this pull request Sep 26, 2024

[v.2.5.0] Release Tracker #135522

Closed

naromero77amd deleted the fix_caffe2_hardcoded_rocm branch September 26, 2024 17:52

njzjz mentioned this pull request Oct 1, 2024

[BUG] Building from source with PyTorch backend on AMD system fails deepmodeling/deepmd-kit#4008

Closed

Conversation

naromero77amd commented Sep 18, 2024 • edited by pytorch-bot bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pytorch-bot bot commented Sep 18, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/136283

❌ 4 New Failures, 6 Unrelated Failures

Uh oh!

naromero77amd commented Sep 18, 2024

Uh oh!

naromero77amd commented Sep 18, 2024

Uh oh!

pytorch-bot bot commented Sep 18, 2024

Uh oh!

pytorch-bot bot commented Sep 18, 2024

Uh oh!

jeffdaily left a comment

Choose a reason for hiding this comment

Uh oh!

naromero77amd commented Sep 20, 2024

Uh oh!

pytorchmergebot commented Sep 20, 2024

Uh oh!

pytorchmergebot commented Sep 20, 2024

Uh oh!

Uh oh!

Uh oh!

atalman left a comment

Choose a reason for hiding this comment

Uh oh!

naromero77amd commented Sep 24, 2024

Uh oh!

jithunnair-amd commented Sep 26, 2024

Uh oh!

pytorchmergebot commented Sep 26, 2024

Merge started

Uh oh!

jithunnair-amd commented Sep 26, 2024

Uh oh!

pytorchbot commented Sep 26, 2024

Cherry picking #136283

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

10 participants

naromero77amd commented Sep 18, 2024 •

edited by pytorch-bot bot

Loading

pytorch-bot bot commented Sep 18, 2024 •

edited

Loading