From 8b6e178aeb0ce2de8a1daa6cdabbadf3ef79b01e Mon Sep 17 00:00:00 2001 From: yasahi-hpc <57478230+yasahi-hpc@users.noreply.github.com> Date: Mon, 7 Oct 2024 19:19:30 +0200 Subject: [PATCH] fix: out of range access in 1D fftshifts (#153) * fix: out of range access in 1D fftshifts * fix: typo * Build OpenMP backend with Debug in CI * Update kokkos-fft version to 0.2.1 * Revert "Update kokkos-fft version to 0.2.1" This reverts commit c490e4aa90348278797abc6428b5ee61d3d6d489. * fix: use extent_int for int extent * install Kokkos with Release in CI * Revert "install Kokkos with Release in CI" This reverts commit 8543675577fa758a9801c84145616605d7e33b74. --------- Co-authored-by: Yuuichi Asahi --- .github/workflows/build_test.yaml | 21 +++++++++++++-------- common/src/KokkosFFT_Helpers.hpp | 16 +++++++++------- 2 files changed, 22 insertions(+), 15 deletions(-) diff --git a/.github/workflows/build_test.yaml b/.github/workflows/build_test.yaml index c1eacd30..8f24697c 100644 --- a/.github/workflows/build_test.yaml +++ b/.github/workflows/build_test.yaml @@ -14,9 +14,6 @@ on: - main env: - # Customize the CMake build type here (Release, Debug, RelWithDebInfo, etc.) - BUILD_TYPE: Release - # Force the use of BuildKit for Docker DOCKER_BUILDKIT: 1 @@ -76,6 +73,7 @@ jobs: kokkos: -DKokkos_ENABLE_SERIAL=ON kokkos_fft: -DCMAKE_CXX_FLAGS="-Wall -Wextra" -DCMAKE_COMPILE_WARNING_AS_ERROR=ON -DCMAKE_CXX_CLANG_TIDY="clang-tidy;-warnings-as-errors=*" benchmark: OFF + cmake_build_type: Debug - name: openmp image: gcc compiler: @@ -86,6 +84,7 @@ jobs: kokkos: -DKokkos_ENABLE_OPENMP=ON -DKokkos_ENABLE_SERIAL=ON kokkos_fft: -DCMAKE_CXX_FLAGS="-Wall -Wextra" -DCMAKE_COMPILE_WARNING_AS_ERROR=ON benchmark: ON + cmake_build_type: Debug - name: threads image: gcc compiler: @@ -96,6 +95,7 @@ jobs: kokkos: -DKokkos_ENABLE_THREADS=ON kokkos_fft: -DCMAKE_CXX_FLAGS="-Wall -Wextra" -DCMAKE_COMPILE_WARNING_AS_ERROR=ON benchmark: ON + cmake_build_type: Release - name: serial image: gcc compiler: @@ -106,6 +106,7 @@ jobs: kokkos: -DKokkos_ENABLE_SERIAL=ON kokkos_fft: -DCMAKE_CXX_FLAGS="-Wall -Wextra" -DCMAKE_COMPILE_WARNING_AS_ERROR=ON benchmark: ON + cmake_build_type: Release - name: cuda image: nvcc compiler: @@ -116,6 +117,7 @@ jobs: kokkos: -DKokkos_ENABLE_CUDA=ON -DKokkos_ARCH_AMPERE80=ON -DKokkos_ENABLE_OPENMP=ON -DKokkos_ENABLE_SERIAL=ON kokkos_fft: -DCMAKE_CXX_FLAGS="-Wall -Wextra -Werror" benchmark: ON + cmake_build_type: Release - name: hip image: rocm compiler: @@ -126,6 +128,7 @@ jobs: kokkos: -DKokkos_ENABLE_HIP=ON -DKokkos_ARCH_VEGA90A=ON -DKokkos_ENABLE_THREADS=ON kokkos_fft: -DCMAKE_CXX_FLAGS="-Wall -Wextra -Werror" benchmark: ON + cmake_build_type: Release - name: rocm image: rocm compiler: @@ -136,6 +139,7 @@ jobs: kokkos: -DKokkos_ENABLE_HIP=ON -DKokkos_ARCH_VEGA90A=ON kokkos_fft: -DCMAKE_CXX_FLAGS="-Wall -Wextra -Werror" -DKokkosFFT_ENABLE_ROCFFT=ON benchmark: ON + cmake_build_type: Release - name: sycl image: intel compiler: @@ -148,6 +152,7 @@ jobs: kokkos: -DKokkos_ENABLE_SYCL=ON -DKokkos_ARCH_INTEL_GEN=ON kokkos_fft: -DCMAKE_CXX_FLAGS="-Wall -Wextra" benchmark: ON + cmake_build_type: Release target: - name: native cmake_flags: "" @@ -187,7 +192,7 @@ jobs: run: | docker run -v ${{ github.workspace }}:/work ghcr.io/kokkos/kokkos-fft/base_${{ matrix.backend.image }}_${{ needs.check_docker_files.outputs.image_suffix }}:${{ needs.check_docker_files.outputs.image_tag }} \ cmake -B build \ - -DCMAKE_BUILD_TYPE=${{ env.BUILD_TYPE }} \ + -DCMAKE_BUILD_TYPE=${{ matrix.backend.cmake_build_type }} \ -DCMAKE_C_COMPILER=${{ matrix.backend.compiler.c }} \ -DCMAKE_CXX_COMPILER=${{ matrix.backend.compiler.cxx }} \ -DCMAKE_CXX_STANDARD=${{ matrix.backend.cmake_flags.cxx_standard }} \ @@ -237,7 +242,7 @@ jobs: run: | docker run -v ${{ github.workspace }}:/work ghcr.io/kokkos/kokkos-fft/base_${{ matrix.backend.image }}_${{ needs.check_docker_files.outputs.image_suffix }}:${{ needs.check_docker_files.outputs.image_tag }} \ cmake -B install_test/as_subdirectory/build \ - -DCMAKE_BUILD_TYPE=${{ env.BUILD_TYPE }} \ + -DCMAKE_BUILD_TYPE=${{ matrix.backend.cmake_build_type }} \ -DCMAKE_C_COMPILER=${{ matrix.backend.compiler.c }} \ -DCMAKE_CXX_COMPILER=${{ matrix.backend.compiler.cxx }} \ -DCMAKE_CXX_STANDARD=${{ matrix.backend.cmake_flags.cxx_standard }} \ @@ -253,7 +258,7 @@ jobs: docker run -v ${{ github.workspace }}:/work ghcr.io/kokkos/kokkos-fft/base_${{ matrix.backend.image }}_${{ needs.check_docker_files.outputs.image_suffix }}:${{ needs.check_docker_files.outputs.image_tag }} \ cmake -B build_kokkos \ -DCMAKE_INSTALL_PREFIX=/work/install \ - -DCMAKE_BUILD_TYPE=${{ env.BUILD_TYPE }} \ + -DCMAKE_BUILD_TYPE=${{ matrix.backend.cmake_build_type }} \ -DCMAKE_C_COMPILER=${{ matrix.backend.compiler.c }} \ -DCMAKE_CXX_COMPILER=${{ matrix.backend.compiler.cxx }} \ -DCMAKE_CXX_STANDARD=${{ matrix.backend.cmake_flags.cxx_standard }} \ @@ -270,7 +275,7 @@ jobs: cmake -B build_kokkos_fft \ -DCMAKE_INSTALL_PREFIX=/work/install\ -DCMAKE_PREFIX_PATH=/work/install \ - -DCMAKE_BUILD_TYPE=${{ env.BUILD_TYPE }} \ + -DCMAKE_BUILD_TYPE=${{ matrix.backend.cmake_build_type }} \ -DCMAKE_C_COMPILER=${{ matrix.backend.compiler.c }} \ -DCMAKE_CXX_COMPILER=${{ matrix.backend.compiler.cxx }} \ -DCMAKE_CXX_STANDARD=${{ matrix.backend.cmake_flags.cxx_standard }} \ @@ -285,7 +290,7 @@ jobs: run: | docker run -v ${{ github.workspace }}:/work ghcr.io/kokkos/kokkos-fft/base_${{ matrix.backend.image }}_${{ needs.check_docker_files.outputs.image_suffix }}:${{ needs.check_docker_files.outputs.image_tag }} \ cmake -B install_test/as_library/build \ - -DCMAKE_BUILD_TYPE=${{ env.BUILD_TYPE }} \ + -DCMAKE_BUILD_TYPE=${{ matrix.backend.cmake_build_type }} \ -DCMAKE_PREFIX_PATH=/work/install \ -DCMAKE_C_COMPILER=${{ matrix.backend.compiler.c }} \ -DCMAKE_CXX_COMPILER=${{ matrix.backend.compiler.cxx }} \ diff --git a/common/src/KokkosFFT_Helpers.hpp b/common/src/KokkosFFT_Helpers.hpp index d44aa48c..be7bea93 100644 --- a/common/src/KokkosFFT_Helpers.hpp +++ b/common/src/KokkosFFT_Helpers.hpp @@ -43,10 +43,10 @@ void roll(const ExecutionSpace& exec_space, ViewType& inout, axis_type<1> shift, axis_type<1>) { // Last parameter is ignored but present for keeping the interface consistent static_assert(ViewType::rank() == 1, "roll: Rank of View must be 1."); - std::size_t n0 = inout.extent(0); + int n0 = inout.extent_int(0); ViewType tmp("tmp", n0); - std::size_t len = (n0 - 1) / 2 + 1; + int len = (n0 - 1) / 2 + 1; auto [_shift0, _shift1, _shift2] = KokkosFFT::Impl::convert_negative_shift(inout, shift.at(0), 0); @@ -56,10 +56,12 @@ void roll(const ExecutionSpace& exec_space, ViewType& inout, axis_type<1> shift, if (shift2 == 0) { Kokkos::parallel_for( "KokkosFFT::roll", - Kokkos::RangePolicy>( - exec_space, 0, len), - KOKKOS_LAMBDA(std::size_t i) { - tmp(i + shift0) = inout(i); + Kokkos::RangePolicy>(exec_space, + 0, len), + KOKKOS_LAMBDA(int i) { + if (i + shift0 < n0) { + tmp(i + shift0) = inout(i); + } if (i + shift1 < n0) { tmp(i) = inout(i + shift1); } @@ -74,7 +76,7 @@ void roll(const ExecutionSpace& exec_space, ViewType& inout, axis_type<2> shift, axis_type axes) { constexpr int DIM0 = 2; static_assert(ViewType::rank() == DIM0, "roll: Rank of View must be 2."); - int n0 = inout.extent(0), n1 = inout.extent(1); + int n0 = inout.extent_int(0), n1 = inout.extent_int(1); ViewType tmp("tmp", n0, n1); [[maybe_unused]] int len0 = (n0 - 1) / 2 + 1;