Skip to content

HIP: document variables, apply patches to older versions, fix build issues#21852

Merged
alalazo merged 6 commits intospack:developfrom
haampie:feature/document-hip-variables-better
Feb 24, 2021
Merged

HIP: document variables, apply patches to older versions, fix build issues#21852
alalazo merged 6 commits intospack:developfrom
haampie:feature/document-hip-variables-better

Conversation

@haampie
Copy link
Copy Markdown
Member

@haampie haampie commented Feb 22, 2021

Since there are many variables being set I thought it would be a good idea to document them better and slightly simplify the logic for external vs not-external.

Supersedes #21822 ping @bvanessen

@haampie haampie force-pushed the feature/document-hip-variables-better branch from c1eb1f6 to dfd2194 Compare February 22, 2021 11:11
@haampie
Copy link
Copy Markdown
Member Author

haampie commented Feb 22, 2021

Also pinging @srekolam

@haampie
Copy link
Copy Markdown
Member Author

haampie commented Feb 22, 2021

Ok, I've also fixed the situation with rocm 3.8.0 a hair: --rocm-path didn't exist in that version yet, so I dropped it, and the patch for getting a proper patch version number in libamdhip64.so applied to 3.8.0 too.

@haampie haampie requested a review from srekolam February 22, 2021 16:20
@haampie haampie changed the title Document hip/package.py better HIP: document variables, apply patches to older versions, fix build issues Feb 22, 2021
@haampie haampie force-pushed the feature/document-hip-variables-better branch from 09ab499 to 74f8f10 Compare February 22, 2021 17:46
Copy link
Copy Markdown
Contributor

@srekolam srekolam left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i would like try this change but what i know is, You should see following with ROCm4.1:
HIP_COMPILER : clang
HIP_PLATFORM : amd
HIP_RUNTIME : rocclr

@haampie
Copy link
Copy Markdown
Member Author

haampie commented Feb 22, 2021

I'm building the following:

spack:
  view: false
  specs:
  - matrix:
    - [sirius@develop+rocm ^openmpi ^openblas threads=openmp arch=linux-centos8-zen]
    - [^[email protected], ^[email protected], ^[email protected], ^[email protected], ^[email protected]]
  packages:
    all:
      variants: amdgpu_target=gfx906 tensile_architecture=gfx906

only waiting for 3.7.0 to finish, but the rest works fine.

@haampie
Copy link
Copy Markdown
Member Author

haampie commented Feb 22, 2021

My packages build fine with 3.7, 3.8, 3.9, 3.10 and 4.0 (which include rocblas & rocfft). Only 3.5 does not work because because it is missing that patch for the shared lib name, but that was the case from the beginning.

So this PR is ready for review now

@bvanessen
Copy link
Copy Markdown
Contributor

@haampie Any idea on when this PR will merge?

@haampie
Copy link
Copy Markdown
Member Author

haampie commented Feb 22, 2021

As soon as @srekolam agrees with it, so soon (tm)

@alalazo alalazo requested a review from srekolam February 23, 2021 10:03
Copy link
Copy Markdown
Contributor

@srekolam srekolam left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i built few packages like rocblas,rocrand , rocfft for @3.7.0 ,@3.8.0, @3.9.0,@4.0.0 successfully with this PR changes.The changes look good .. @haampie ,thanks!!!

@alalazo alalazo merged commit 4a9c8ec into spack:develop Feb 24, 2021
@haampie haampie deleted the feature/document-hip-variables-better branch February 24, 2021 09:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants