Bug 2213979 - Review Request: rocrand - ROCm random number generator
Summary: Review Request: rocrand - ROCm random number generator
Keywords:
Status: CLOSED RAWHIDE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Tom Rix
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2023-06-10 15:52 UTC by Jeremy Newton
Modified: 2023-06-11 17:09 UTC (History)
3 users (show)

Fixed In Version:
Doc Type: If docs needed, set a value
Doc Text:
Clone Of:
Environment:
Last Closed: 2023-06-11 17:09:02 UTC
Type: ---
Embargoed:
trix: fedora-review+


Attachments (Terms of Use)

Description Jeremy Newton 2023-06-10 15:52:24 UTC
Spec URL: https://mystro256.fedorapeople.org/rocrand.spec
SRPM URL: https://mystro256.fedorapeople.org/rocrand-5.5.1-1.fc39.src.rpm
Description:
The rocRAND project provides functions that generate pseudo-random and
quasi-random numbers.
The rocRAND library is implemented in the HIP programming language and
optimised for AMD's latest discrete GPUs. It is designed to run on top of AMD's
Radeon Open Compute ROCm runtime, but it also works on CUDA enabled GPUs.
Fedora Account System Username: mystro256
copr build: https://copr.fedorainfracloud.org/coprs/mystro256/rocm-hip/build/6059407/
Note: rpmlint complains that libhiprand doesn't link against glibc, but I believe this is expected with hip libs.

Comment 1 Jeremy Newton 2023-06-10 22:23:48 UTC
For reference, I hope to use this as a template to get the rest of the ROCm Software Platform bits into Fedora.

Comment 2 Jeremy Newton 2023-06-10 22:26:28 UTC
x86_64 is the only thing I could get building, likely because upstream only officially supports this arch.
It would be nice to extend it to aarch64 and ppc64le, as I've done with all of the existing ROCm bits in Fedora but they are low priority.

Comment 3 Tom Rix 2023-06-10 23:38:23 UTC
i am fine with just x86_64 for now

some comments on the spec filefedora > diff -u rocrand.spec rocrand.spec.review 
--- rocrand.spec	2023-06-08 17:00:00.000000000 -0700
+++ rocrand.spec.review	2023-06-10 16:34:21.834345933 -0700
@@ -4,15 +4,19 @@
 %global rocm_patch 1
 %global rocm_version %{rocm_release}.%{rocm_patch}
 
-%undefine _hardened_build
+# Explain, commenting them out and still builds
+# %undefine _hardened_build
 # Compiler is hipcc, which is clang based:
 %global toolchain clang
 #The target doesn't support -fcf-protection:
-%global optflags %(o="%{optflags}"; echo ${o//-fcf-protection/})
+# %global optflags %(o="%{optflags}"; echo ${o//-fcf-protection/})
+
+# Nice to have a check
+%bcond_with check
 
 Name:           rocrand
 Version:        %{rocm_version}
-Release:        1%{?dist}
+Release:        2%{?dist}
 Summary:        ROCm random number generator
 
 Url:            https://github.com/ROCmSoftwarePlatform
@@ -25,6 +29,10 @@
 BuildRequires:  cmake
 BuildRequires:  compiler-rt
 BuildRequires:  clang-devel
+# rocrand-devel.x86_64: W: no-documentation
+# hiprand-devel.x86_64: W: no-documentation
+# no docs warnings, add build requires till you get some autogen-ed stuff
+BuildRequires:  doxygen
 BuildRequires:  glibc-headers
 BuildRequires:  lld
 BuildRequires:  llvm-devel
@@ -32,6 +40,9 @@
 BuildRequires:  rocm-comgr-devel
 BuildRequires:  rocm-hip-devel
 BuildRequires:  rocm-runtime-devel
+%if %{with check}
+# Add some stuff
+%endif
 
 # Only x86_64 works right now:
 ExclusiveArch:  x86_64
@@ -45,7 +56,9 @@
 Radeon Open Compute ROCm runtime, but it also works on CUDA enabled GPUs.
 
 %package devel
-Summary:        rocRAND development package
+# rocrand-devel.x86_64: W: summary-not-capitalized rocRAND development package
+# Summary:        rocRAND development package
+Summary:        The rocRAND development package
 Requires:       %{name}%{?_isa} = %{version}-%{release}
 
 %description devel
@@ -62,7 +75,8 @@
 backend. Currently, hipRAND supports either rocRAND or cuRAND.
 
 %package -n hiprand-devel
-Summary:        hipRAND development package
+#Summary:        hipRAND development package
+Summary:        The hipRAND development package
 Requires:       hiprand%{?_isa} = %{version}-%{release}
 
 %description -n hiprand-devel
@@ -75,19 +89,34 @@
 mv hipRAND-rocm-%{version}/* hipRAND/
 
 %build
-export CXX=hipcc
+# use cmake option over export
+# export CXX=hipcc
+# Reordered the options in alpha order
 %cmake \
-        -DROCM_SYMLINK_LIBS=OFF \
-        -DCMAKE_INSTALL_LIBDIR=%{_lib} \
-        -DBUILD_FILE_REORG_BACKWARD_COMPATIBILITY=OFF
+    -DBUILD_FILE_REORG_BACKWARD_COMPATIBILITY=OFF \
+%if %{with check}
+    -DBUILD_TEST=ON \
+%endif    
+    -DCMAKE_INSTALL_LIBDIR=%{_lib} \
+    -DCMAKE_CXX_COMPILER=hipcc \
+    -DROCM_SYMLINK_LIBS=OFF
+
 %cmake_build
 
+# takes a long time and needs a gpu..
+%if %{with check}
+%ctest
+%endif
+
+
 %install
 %cmake_install
+# why ?
 chrpath --delete %{buildroot}/%{_libdir}/libhiprand.so.1.*
 
 %files
-%doc CHANGELOG.md
+# Use readme over changelog
+%doc README.md
 %license LICENSE.txt
 %{_libdir}/lib%{name}.so.1{,.*}
 %exclude %{_docdir}/%{name}/LICENSE.txt
@@ -98,7 +127,7 @@
 %{_libdir}/cmake/%{name}
 
 %files -n hiprand
-%doc hipRAND/CHANGELOG.md
+%doc hipRAND/README.md
 %license hipRAND/LICENSE.txt
 %{_libdir}/libhiprand.so.1{,.*}
 
@@ -108,5 +137,8 @@
 %{_libdir}/cmake/hiprand
 
 %changelog
+* Sat Jun 10 2023 Tom Rix <trix> - 5.5.1-2
+- Review
+
 * Fri Jun 09 2023 Jeremy Newton <alexjnewt at hotmail dot com> - 5.5.1-1
 - Initial package

Comment 4 Jeremy Newton 2023-06-11 01:23:02 UTC
> # Explain, commenting them out and still builds

For me the hardened flags causes this issue:

    fatal error: error in backend: Cannot select: 0x71af480: i64 = FrameIndex<0>
    In function: _ZN12rocrand_host6detail15generate_kernelIj28rocrand_poisson_distributionIL23rocrand_discrete_method1ELb0EEEEvPNS0_19mt19937_octo_engineEPT_mT0_
    clang-16: error: clang frontend command failed with exit code 70 (use -v to see invocation)

Are you saying it works for you? The Debian guys disable the hardening too, so I took it at face value.

> # %global optflags %(o="%{optflags}"; echo ${o//-fcf-protection/})

This also doesn't work, as it causes this error for me:

    error: option 'cf-protection=return' cannot be specified on this target

> # why ?

I changed it to a sed patch in %prep if that's better.

Update:
Spec URL: https://mystro256.fedorapeople.org/rocrand.spec
SRPM URL: https://mystro256.fedorapeople.org/rocrand-5.5.1-2.fc39.src.rpm
copr build: https://copr.fedorainfracloud.org/coprs/mystro256/rocm-hip/build/6059869/

Comment 5 Jeremy Newton 2023-06-11 01:27:02 UTC
> copr build: https://copr.fedorainfracloud.org/coprs/mystro256/rocm-hip/build/6059869/

Ignore the non-x86_64 builds, I built them by accident... I would be nice if copr could autodetect exclusive arch.

Comment 6 Tom Rix 2023-06-11 12:58:44 UTC
i assumed doc's would be handled by cmake, looks like there is a custom makefile
remove the doxygen from build depends
we can add the docs when upstream catches up

when --with check the rpms are in a bad state

[root@fedora x86_64]# rpm -ihv rocrand-5.5.1-2.fc39.x86_64.rpm 
error: Failed dependencies:
	libhiprand.so.1()(64bit) is needed by rocrand-5.5.1-2.fc39.x86_64
[root@fedora x86_64]# rpm -ihv hiprand-5.5.1-2.fc39.x86_64.rpm 
error: Failed dependencies:
	librocrand.so.1()(64bit) is needed by hiprand-5.5.1-2.fc39.x86_64

my guess is you need to move the tests to hiprand package
both issues are minor.
Approved.

Comment 7 Jeremy Newton 2023-06-11 14:37:09 UTC
Yeah I was told by some of the ROCm guys that the doxygen is a WIP, so it should come eventually.

> my guess is you need to move the tests to hiprand package

Sounds good, I'll fix it before upload. I need to either move the tests to hiprand, or properly split the tests in half between the packages.

Thanks again for the review! If you want to package rocBLAS or similar, feel free to take my spec as a template and CC me to the review.

Comment 8 Fedora Admin user for bugzilla script actions 2023-06-11 14:42:08 UTC
The Pagure repository was created at https://src.fedoraproject.org/rpms/rocrand

Comment 9 Tom Rix 2023-06-11 16:16:52 UTC
I would like to rocBLAS going however, it depends on Tensile.
Running some of the tensile_rocblas_* apps have problems.
tensile_rocblas_cgemm show this problem

b'-- hip::amdhip64 is SHARED_LIBRARY\nLLVMObjectYAML_LIBRARY: /usr/lib64/libLLVMObjectYAML.a\nCMake Error at /usr/share/cmake/Modules/FindPackageHandleStandardArgs.cmake:230 (message):\n  Could NOT find ROCmSMI (missing: ROCM_SMI_LIBRARY ROCM_SMI_ROOT)\nCall Stack (most recent call first):\n  /usr/share/cmake/Modules/FindPackageHandleStandardArgs.cmake:600 (_FPHSA_FAILURE_MESSAGE)\n  cmake/FindROCmSMI.cmake:46 (find_package_handle_standard_args)\n  client/CMakeLists.txt:64 (find_package)\n\n\n-- Configuring incomplete, errors occurred!\n'
Traceback (most recent call last):

so rocmsmi should be next, but i have not looked at this yet.

Comment 10 Jeremy Newton 2023-06-11 16:29:03 UTC
Yeah rocmsmi is in fedora now as "rocm-smi", but the project was completely rewritten as "rocm-smi-lib" upstream, so the Fedora developer gave up on it:
https://github.com/RadeonOpenCompute/rocm_smi_lib

Feel free to take that if you have time, as I've been too tangled up with the other bits to get to it.

Comment 11 Jeremy Newton 2023-06-11 17:09:02 UTC
Built in Rawhide


Note You need to log in before you can comment on or make changes to this bug.