Skip to content

GPU package bugfix / cmake improvement#3674

Merged
akohlmey merged 3 commits intolammps:developfrom
rbberger:gpu_rocm_bugfix
Mar 7, 2023
Merged

GPU package bugfix / cmake improvement#3674
akohlmey merged 3 commits intolammps:developfrom
rbberger:gpu_rocm_bugfix

Conversation

@rbberger
Copy link
Copy Markdown
Member

@rbberger rbberger commented Mar 6, 2023

Summary

  • detect hipcub via CMake, and take advantage of the hip::host and hip::hipcub targets. we don't want to assume where header include path is.
  • fixup broken HIP code path in GPU package. num_atoms was not defined in case of USE_HIP_DEVICE_SORT.
  • drop hcc as HIP_PLATFORM, any modern ROCm install will use amd (only 5.1+ are worth looking at).

Related Issue(s)

Author(s)

@rbberger

Licensing

By submitting this pull request, I agree, that my contribution will be included in LAMMPS and redistributed under either the GNU General Public License version 2 (GPL v2) or the GNU Lesser General Public License version 2.1 (LGPL v2.1).

Backward Compatibility

Implementation Notes

Bug was introduced by 4244d2e

Post Submission Checklist

  • The feature or features in this pull request is complete
  • Licensing information is complete
  • Corresponding author information is complete
  • The source code follows the LAMMPS formatting guidelines
  • Suitable new documentation files and/or updates to the existing docs are included
  • The added/updated documentation is integrated and tested with the documentation build system
  • The feature has been verified to work with the conventional build system
  • The feature has been verified to work with the CMake based build system
  • Suitable tests have been added to the unittest tree.
  • A package specific README file has been included or updated
  • One or more example input decks are included

Further Information, Files, and Links

@skyreflectedinmirrors
Copy link
Copy Markdown
Collaborator

LGTM, might be able to simplify further by inheriting compile options from hip::host

@rbberger
Copy link
Copy Markdown
Member Author

rbberger commented Mar 6, 2023

LGTM, might be able to simplify further by inheriting compile options from hip::host

You're right. (Quite the challenge to find the right repo where this file comes from): https://github.com/ROCm-Developer-Tools/hipamd/blob/develop/hip-config.cmake.in#L180.

@skyreflectedinmirrors
Copy link
Copy Markdown
Collaborator

Agreed -- quick reference is:

  • hipamd -> the AMD backend of HIP, also has forwarding for NV backend
  • HIP -> headers, that theoretically multiple backends could implement

HIP owns the HIP-Language cmake: https://github.com/ROCm-Developer-Tools/HIP/blob/develop/hip-lang-config.cmake.in#L81, which is what gets turned off if you used (e.g.,) enable_language(HIP) for CMake >= 3.21(and/or setting the language)

@akohlmey akohlmey merged commit 6d7d0f7 into lammps:develop Mar 7, 2023
@akohlmey akohlmey deleted the gpu_rocm_bugfix branch March 7, 2023 12:39
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.

4 participants