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

Develop Stream 2024-05-06 implementation #115

Merged
merged 8 commits into from
Jun 3, 2024

Conversation

Beanavil
Copy link
Collaborator

@Beanavil Beanavil commented May 6, 2024

This PR contains the commits that implement new examples/features. The changes introduced in this PR can be summarized into:

  • Added copyright check script
  • Implemented minimal CMake CUDA example
  • Added a default case for externally controlled enumerations in hipSOLVER and rocSPARSE common utility files
  • Enabled the use of NVIDIA platform on CMake HIP language, supported by the 3.28 release of CMake (part I)
  • Added a consistent error code when reporting examples' failure
  • Did some internal CI improvements

@Beanavil Beanavil requested review from Naraenda and samjwu May 6, 2024 14:38
@Beanavil Beanavil requested review from a team and dgaliffiAMD as code owners May 6, 2024 14:38
.githooks/install Show resolved Hide resolved
Common/HipPlatform.cmake Outdated Show resolved Hide resolved
HIP-Basic/hello_world_cuda/CMakeLists.txt Outdated Show resolved Hide resolved
HIP-Basic/hello_world_cuda/main.hip Outdated Show resolved Hide resolved
Libraries/hipBLAS/her/main.hip Outdated Show resolved Hide resolved
Libraries/rocBLAS/level_1/nrm2/main.cpp Outdated Show resolved Hide resolved
Libraries/rocBLAS/level_1/scal/main.cpp Outdated Show resolved Hide resolved
Libraries/rocBLAS/level_2/gemv/main.cpp Outdated Show resolved Hide resolved
Libraries/rocBLAS/level_2/her/main.cpp Outdated Show resolved Hide resolved

function(select_gpu_language)
if (DEFINED CACHE{GPU_RUNTIME} AND NOT "${ROCM_EXAMPLES_GPU_RUNTIME_DISABLE_WARN}")
message(DEPRECATION "GPU_RUNTIME is deprecated, please use ROCM_EXAMPLES_GPU_LANGUAGE to set"
Copy link
Collaborator

Choose a reason for hiding this comment

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

After this change, we see ~300 cmake warnings. I think we should clean up the remaining examples before finally merging this PR.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is because there are still two phases yet to be implemented until the feature is complete (that is, until the examples can be used with the NVIDIA platform of CMake's hip language support). We can drop this commit for now and push the feature when it is fully implemented if that's better.

Copy link
Collaborator

Choose a reason for hiding this comment

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

We can keep the PR open until subsequent parts are done. I just think it would be a bad user experience to anyone trying to build the examples and then seeing all these warnings.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Well, the second and third phases of this feature will take quite a long time. I just dropped this commit to not drag this PR further, and we'll upstream it when finished.

@Beanavil Beanavil force-pushed the develop_stream_2024_05_06_impl branch from c337f46 to 494ce11 Compare May 16, 2024 15:29
@dgaliffiAMD
Copy link
Collaborator

Hi @Beanavil ,
Is this change also addressing: #35 ?

@Beanavil Beanavil force-pushed the develop_stream_2024_05_06_impl branch from 494ce11 to 12e8fad Compare May 27, 2024 12:01
@Beanavil
Copy link
Collaborator Author

@dgaliffiAMD regarding this question, there was a commit implementing part of that issue, as we internally started to work on that. But we designed internally 3 phases to implement the whole feature, and only the first of them is done as of now.

@dgaliffiAMD dgaliffiAMD self-requested a review May 27, 2024 15:42
@Beanavil Beanavil force-pushed the develop_stream_2024_05_06_impl branch from 12e8fad to 8a73fca Compare May 28, 2024 07:55
@dgaliffiAMD dgaliffiAMD merged commit fd90d0e into ROCm:develop Jun 3, 2024
2 checks passed
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

Successfully merging this pull request may close these issues.

5 participants