Use std::aligned_alloc for allocations #6341

masterleinad · 2023-08-07T19:13:46Z

Related to #5011. It seems that we don't need all these different allocation mechanisms anymore and can use std::aligned_alloc(https://en.cppreference.com/w/cpp/memory/c/aligned_alloc) instead. Also, there were some problems with INTEL_MM_ALLOC reported that don't appear anymore when using std::malloc instead.
Note that MSVC is unable to handle aligned allocations of any kind and provides _aligned_malloc (to be freed with _aligned_free) instead.

This pull request deprecates all other allocation variants for HostSpace to only use aligned_malloc.

HBWSpace is untested and broken, see #4889. I would address it in a different pull request (maybe that one).

core/src/Kokkos_HostSpace.hpp

cz4rs

Looks good to me 👍

masterleinad · 2023-08-08T17:05:14Z

https://github.com/zippylab should be able to comment some more on the issue found with using _mm_malloc.

zippylab · 2023-08-08T20:45:27Z

https://github.com/zippylab should be able to comment some more on the issue found with using _mm_malloc.

We had been seeing crashes of the XGC application on the Sunspot system (Aurora test system) coming from this line in HostSpace.cpp (in Kokkos::HostSpace::impl_deallocate):

(gdb) list
212           void *alloc_ptr = *(reinterpret_cast<void **>(arg_alloc_ptr) - 1);
/213           free(alloc_ptr);
214         }
215     #if defined(KOKKOS_ENABLE_INTEL_MM_ALLOC)
216         else if (m_alloc_mech == INTEL_MM_ALLOC) {
217           _mm_free(arg_alloc_ptr);  <<<<<<<<<<<<<<<<<<HERE<<<<<<
218         }
219     #endif
220       }
221     }
(gdb)

in a part of XGC that is copying particle data from device to host memory (using Cabana::SoA and Cabana::AoSoA).

After rebuilding Kokkos, Cabana, and XGC with the changes in this PR, that failure mode of XGC went away, and it ran past the point where the SEGV was happening before in the same test case.

dalg24

What about RawMemoryAllocationFailure::AllocationMechanism?
Should we not deprecate enumerators that are not supported?

core/src/impl/Kokkos_HostSpace.cpp

dalg24 · 2023-08-09T13:13:02Z

core/src/impl/Kokkos_HostSpace.cpp

+    size_t size_alignment_multiple =
+        (arg_alloc_size + alignment - 1) / alignment * alignment;
+#ifdef KOKKOS_COMPILER_MSVC
+    ptr = _aligned_malloc(size_alignment_multiple, alignment);


Does _aligned_malloc also require the size argument to be a multiple of the alignment?

I couldn't find information about that but preferred doing it anyway for consistency.

Does MSVC not support aligned_alloc?

MSVC indeed does not support aligned_alloc as noted on CppReference.com

It would be nice to be able to use operator new(size_alignment_multiple, std::align_val_t(alignment)) instead, avoiding the conditional, but then we'd have to store the alignment to pass to the corresponding operator delete

core/src/OpenMP/Kokkos_OpenMP_Instance.cpp

core/src/Kokkos_HostSpace.hpp

masterleinad · 2023-08-09T15:58:20Z

What about RawMemoryAllocationFailure::AllocationMechanism?
Should we not deprecate enumerators that are not supported?

Sure, see 8c8c9c3.

crtrott

I don't think the print stuff works?

crtrott · 2023-08-09T22:37:22Z

core/src/impl/Kokkos_HostSpace.cpp

+    size_t size_alignment_multiple =
+        (arg_alloc_size + alignment - 1) / alignment * alignment;
+#ifdef KOKKOS_COMPILER_MSVC
+    ptr = _aligned_malloc(size_alignment_multiple, alignment);


Does MSVC not support aligned_alloc?

crtrott · 2023-08-09T22:39:35Z

core/src/impl/Kokkos_Error.cpp

@@ -102,11 +102,13 @@ void Experimental::RawMemoryAllocationFailure::print_error_message(
  o << "  (The allocation mechanism was ";
  switch (m_mechanism) {
    case AllocationMechanism::StdMalloc: o << "standard malloc()."; break;


How does this work if KOKKOS_ENABLE_DEPRECATED_CODE_4 is off? Isn't the entire AllocationMechanism thing removed if DEPRECATED_CODE_4 is not enabled?

You are looking at the wrong place, the relevant is core/src/impl/Kokkos_Error.hpp, not core/src/Kokkos_HostSpace.hpp. Both files contain AllocationMechanism enums but they are different. Note that we test with KOKKOS_DEPRECATED_CODE_4=OFF in our CI, see

kokkos/.jenkins

Lines 56 to 92 in 1a3ea28

stage('CUDA-11.7-NVHPC') {

agent {

dockerfile {

filename 'Dockerfile.nvhpc'

dir 'scripts/docker'

additionalBuildArgs '--build-arg BASE=nvcr.io/nvidia/nvhpc:22.9-devel-cuda11.7-ubuntu20.04'

label 'nvidia-docker && large_images'

args '-v /tmp/ccache.kokkos:/tmp/ccache --env NVIDIA_VISIBLE_DEVICES=$NVIDIA_VISIBLE_DEVICES'

}

}

environment {

OMP_NUM_THREADS = 8

// Nested OpenMP does not work for this configuration,

// so disabling it

OMP_MAX_ACTIVE_LEVELS = 1

OMP_PLACES = 'threads'

OMP_PROC_BIND = 'spread'

NVHPC_CUDA_HOME = '/opt/nvidia/hpc_sdk/Linux_x86_64/22.9/cuda/11.7'

}

steps {

sh '''rm -rf build && mkdir -p build && cd build && \

/opt/cmake/bin/cmake \

-DCMAKE_BUILD_TYPE=RelWithDebInfo \

-DCMAKE_CXX_COMPILER=nvc++ \

-DCMAKE_CXX_STANDARD=17 \

-DCMAKE_CXX_FLAGS="--diag_suppress=implicit_return_from_non_void_function,no_device_stack" \

-DKokkos_ARCH_NATIVE=ON \

-DKokkos_ENABLE_COMPILER_WARNINGS=ON \

-DKokkos_ENABLE_DEPRECATED_CODE_4=OFF \

-DKokkos_ENABLE_TESTS=ON \

-DKokkos_ENABLE_CUDA=ON \

-DKokkos_ENABLE_CUDA_LAMBDA=ON \

-DKokkos_ENABLE_OPENMP=ON \

.. && \

make -j8 && ctest --verbose'''

}

}

for example.

Ok that seems somewhat unfortunate naming but ok.

masterleinad · 2023-08-10T13:03:34Z

Note that we get a couple of warnings (if we enable KOKKOS_DEPRECATED_CODE_4 and deprecation warnings; here with warnings as errors)

/tmp/kokkos/core/src/impl/Kokkos_Error.cpp: In member function 'void Kokkos::Experimental::RawMemoryAllocationFailure::print_error_message(std::ostream&) const':
/tmp/kokkos/core/src/impl/Kokkos_Error.cpp:106:31: error: 'Kokkos::Experimental::RawMemoryAllocationFailure::AllocationMechanism::PosixMemAlign' is deprecated [-Werror=deprecated-declarations]
  106 |     case AllocationMechanism::PosixMemAlign: o << "posix_memalign()."; break;
      |                               ^~~~~~~~~~~~~
In file included from /tmp/kokkos/core/src/impl/Kokkos_Error.cpp:28:
/tmp/kokkos/core/src/impl/Kokkos_Error.hpp:117:5: note: declared here
  117 |     PosixMemAlign KOKKOS_DEPRECATED,
      |     ^~~~~~~~~~~~~
/tmp/kokkos/core/src/impl/Kokkos_Error.cpp:106:31: error: 'Kokkos::Experimental::RawMemoryAllocationFailure::AllocationMechanism::PosixMemAlign' is deprecated [-Werror=deprecated-declarations]
  106 |     case AllocationMechanism::PosixMemAlign: o << "posix_memalign()."; break;
      |                               ^~~~~~~~~~~~~
/tmp/kokkos/core/src/impl/Kokkos_Error.hpp:117:5: note: declared here
  117 |     PosixMemAlign KOKKOS_DEPRECATED,
      |     ^~~~~~~~~~~~~
/tmp/kokkos/core/src/impl/Kokkos_Error.cpp:107:31: error: 'Kokkos::Experimental::RawMemoryAllocationFailure::AllocationMechanism::PosixMMap' is deprecated [-Werror=deprecated-declarations]
  107 |     case AllocationMechanism::PosixMMap: o << "POSIX mmap()."; break;
      |                               ^~~~~~~~~
/tmp/kokkos/core/src/impl/Kokkos_Error.hpp:118:5: note: declared here
  118 |     PosixMMap KOKKOS_DEPRECATED,
      |     ^~~~~~~~~
/tmp/kokkos/core/src/impl/Kokkos_Error.cpp:107:31: error: 'Kokkos::Experimental::RawMemoryAllocationFailure::AllocationMechanism::PosixMMap' is deprecated [-Werror=deprecated-declarations]
  107 |     case AllocationMechanism::PosixMMap: o << "POSIX mmap()."; break;
      |                               ^~~~~~~~~
/tmp/kokkos/core/src/impl/Kokkos_Error.hpp:118:5: note: declared here
  118 |     PosixMMap KOKKOS_DEPRECATED,
      |     ^~~~~~~~~
/tmp/kokkos/core/src/impl/Kokkos_Error.cpp:108:31: error: 'Kokkos::Experimental::RawMemoryAllocationFailure::AllocationMechanism::IntelMMAlloc' is deprecated [-Werror=deprecated-declarations]
  108 |     case AllocationMechanism::IntelMMAlloc:
      |                               ^~~~~~~~~~~~
/tmp/kokkos/core/src/impl/Kokkos_Error.hpp:119:5: note: declared here
  119 |     IntelMMAlloc KOKKOS_DEPRECATED,
      |     ^~~~~~~~~~~~
/tmp/kokkos/core/src/impl/Kokkos_Error.cpp:108:31: error: 'Kokkos::Experimental::RawMemoryAllocationFailure::AllocationMechanism::IntelMMAlloc' is deprecated [-Werror=deprecated-declarations]
  108 |     case AllocationMechanism::IntelMMAlloc:
      |                               ^~~~~~~~~~~~
/tmp/kokkos/core/src/impl/Kokkos_Error.hpp:119:5: note: declared here
  119 |     IntelMMAlloc KOKKOS_DEPRECATED,
      |     ^~~~~~~~~~~~
cc1plus: all warnings being treated as errors
make[2]: *** [core/src/CMakeFiles/kokkoscore.dir/impl/Kokkos_Error.cpp.o] Error 1
make[2]: *** Waiting for unfinished jobs....
/tmp/kokkos/core/src/impl/Kokkos_HostSpace.cpp:59:60: error: 'AllocationMechanism' is deprecated [-Werror=deprecated-declarations]
   59 | HostSpace::HostSpace(const HostSpace::AllocationMechanism &) : HostSpace() {}
      |                                                            ^
In file included from /tmp/kokkos/core/src/impl/Kokkos_HostSpace.cpp:49:
/tmp/kokkos/core/src/Kokkos_HostSpace.hpp:78:26: note: declared here
   78 |   enum KOKKOS_DEPRECATED AllocationMechanism {
      |                          ^~~~~~~~~~~~~~~~~~~

but they all originate from "*.cc" files.

dalg24

What about HBWSpace?

core/src/impl/Kokkos_HostSpace.cpp

core/src/impl/Kokkos_Error.cpp

core/src/OpenMP/Kokkos_OpenMP_Instance.cpp

core/src/impl/Kokkos_HostSpace.cpp

masterleinad · 2023-08-10T13:32:42Z

What about HBWSpace?

HBWSpace is untested and broken, see #4889. I would address it in a different pull request (maybe that one).

dalg24 · 2023-08-10T13:38:26Z

What about HBWSpace?

HBWSpace is untested and broken, see #4889. I would address it in a different pull request (maybe that one).

Add a note in the PR description that you intentionally did not update HBWSpace.

masterleinad · 2023-08-10T13:43:28Z

Add a note in the PR description that you intentionally did not update HBWSpace.

Here you go!

core/src/impl/Kokkos_Error.hpp

…ses in switch statement

barracuda156 · 2023-08-18T04:47:27Z

@masterleinad @dalg24 @crtrott Unfortunately, this does not work for all platforms: #6367

masterleinad force-pushed the use_aligned_alloc branch from 1093348 to 4b5c2a6 Compare August 7, 2023 19:19

PhilMiller reviewed Aug 7, 2023

View reviewed changes

core/src/Kokkos_HostSpace.hpp Show resolved Hide resolved

masterleinad force-pushed the use_aligned_alloc branch 3 times, most recently from b507200 to 5854e75 Compare August 7, 2023 22:59

Use std::aligned_alloc for allocations

4fbee6a

masterleinad force-pushed the use_aligned_alloc branch from 5854e75 to 4fbee6a Compare August 8, 2023 02:59

masterleinad marked this pull request as ready for review August 8, 2023 11:35

cz4rs approved these changes Aug 8, 2023

View reviewed changes

dalg24 reviewed Aug 9, 2023

View reviewed changes

masterleinad added 2 commits August 9, 2023 11:56

Deprecate unsupported AllocationMechanism enum values

8c8c9c3

Fix typo

2761589

crtrott requested changes Aug 9, 2023

View reviewed changes

masterleinad requested a review from crtrott August 10, 2023 02:53

Remove unused variable

7a4e367

dalg24 reviewed Aug 10, 2023

View reviewed changes

core/src/impl/Kokkos_HostSpace.cpp Outdated Show resolved Hide resolved

core/src/impl/Kokkos_Error.cpp Outdated Show resolved Hide resolved

core/src/OpenMP/Kokkos_OpenMP_Instance.cpp Show resolved Hide resolved

core/src/impl/Kokkos_HostSpace.cpp Show resolved Hide resolved

size_alignment_multiple -> size

9373c47

masterleinad requested a review from dalg24 August 10, 2023 13:52

Remove unused RawMemoryAllocationFailure::AllocationMechanism enums

fa90d1c

masterleinad mentioned this pull request Aug 10, 2023

CHANGELOG: 4.2.0 #6197

Closed

dalg24 reviewed Aug 11, 2023

View reviewed changes

core/src/impl/Kokkos_Error.hpp Show resolved Hide resolved

Deprecate AllocationMechanism enums instead and handle unsupported ca…

09a14d8

…ses in switch statement

crtrott approved these changes Aug 11, 2023

View reviewed changes

dalg24 merged commit 8d8b24a into kokkos:develop Aug 11, 2023
25 of 28 checks passed

dalg24 mentioned this pull request Aug 24, 2023

Add deprecated attribute to HostSpace(AllocationMechanism) definition #6382

Merged

dalg24 mentioned this pull request Feb 8, 2024

Drop KOKKOS_ENABLE_INTEL_MM_ALLOC macro #6797

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use std::aligned_alloc for allocations #6341

Use std::aligned_alloc for allocations #6341

masterleinad commented Aug 7, 2023 •

edited

cz4rs left a comment

masterleinad commented Aug 8, 2023 •

edited

zippylab commented Aug 8, 2023

dalg24 left a comment

dalg24 Aug 9, 2023

masterleinad Aug 9, 2023

crtrott Aug 9, 2023

PhilMiller Aug 10, 2023

masterleinad commented Aug 9, 2023 •

edited

crtrott left a comment

crtrott Aug 9, 2023

crtrott Aug 9, 2023

masterleinad Aug 10, 2023

crtrott Aug 11, 2023

masterleinad commented Aug 10, 2023

dalg24 left a comment

masterleinad commented Aug 10, 2023

dalg24 commented Aug 10, 2023

masterleinad commented Aug 10, 2023

barracuda156 commented Aug 18, 2023

	stage('CUDA-11.7-NVHPC') {
	agent {
	dockerfile {
	filename 'Dockerfile.nvhpc'
	dir 'scripts/docker'
	additionalBuildArgs '--build-arg BASE=nvcr.io/nvidia/nvhpc:22.9-devel-cuda11.7-ubuntu20.04'
	label 'nvidia-docker && large_images'
	args '-v /tmp/ccache.kokkos:/tmp/ccache --env NVIDIA_VISIBLE_DEVICES=$NVIDIA_VISIBLE_DEVICES'
	}
	}
	environment {
	OMP_NUM_THREADS = 8
	// Nested OpenMP does not work for this configuration,
	// so disabling it
	OMP_MAX_ACTIVE_LEVELS = 1
	OMP_PLACES = 'threads'
	OMP_PROC_BIND = 'spread'
	NVHPC_CUDA_HOME = '/opt/nvidia/hpc_sdk/Linux_x86_64/22.9/cuda/11.7'
	}
	steps {
	sh '''rm -rf build && mkdir -p build && cd build && \
	/opt/cmake/bin/cmake \
	-DCMAKE_BUILD_TYPE=RelWithDebInfo \
	-DCMAKE_CXX_COMPILER=nvc++ \
	-DCMAKE_CXX_STANDARD=17 \
	-DCMAKE_CXX_FLAGS="--diag_suppress=implicit_return_from_non_void_function,no_device_stack" \
	-DKokkos_ARCH_NATIVE=ON \
	-DKokkos_ENABLE_COMPILER_WARNINGS=ON \
	-DKokkos_ENABLE_DEPRECATED_CODE_4=OFF \
	-DKokkos_ENABLE_TESTS=ON \
	-DKokkos_ENABLE_CUDA=ON \
	-DKokkos_ENABLE_CUDA_LAMBDA=ON \
	-DKokkos_ENABLE_OPENMP=ON \
	.. && \
	make -j8 && ctest --verbose'''
	}
	}

Use std::aligned_alloc for allocations #6341

Use std::aligned_alloc for allocations #6341

Conversation

masterleinad commented Aug 7, 2023 • edited

cz4rs left a comment

Choose a reason for hiding this comment

masterleinad commented Aug 8, 2023 • edited

zippylab commented Aug 8, 2023

dalg24 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

masterleinad commented Aug 9, 2023 • edited

crtrott left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

masterleinad commented Aug 10, 2023

dalg24 left a comment

Choose a reason for hiding this comment

masterleinad commented Aug 10, 2023

dalg24 commented Aug 10, 2023

masterleinad commented Aug 10, 2023

barracuda156 commented Aug 18, 2023

masterleinad commented Aug 7, 2023 •

edited

masterleinad commented Aug 8, 2023 •

edited

masterleinad commented Aug 9, 2023 •

edited