Fix Kokkos_SIMD with AVX2 on 64-bit architectures

[masterleinad]

Fixes #6070. For better test coverage (and convenience for the user?), this pull request also enables the avx2 overloads if __AVX2__ is defined (which should be the case when the user sets Kokkos_ARCH_AVX2 but also for Kokkos_ARCH_NATIVE=ON).

[ibaned]

Does AVX2 get enabled if the user sets Kokkos_ARCH_BDW=ON?

[masterleinad]

Does AVX2 get enabled if the user sets Kokkos_ARCH_BDW=ON?

Yes,

kokkos/cmake/kokkos_arch.cmake

Lines 366 to 376 in c09dd1c

	IF (KOKKOS_ARCH_BDW)
	SET(KOKKOS_ARCH_AVX2 ON)
	COMPILER_SPECIFIC_FLAGS(
	COMPILER_ID KOKKOS_CXX_HOST_COMPILER_ID
	Cray NO-VALUE-SPECIFIED
	Intel -xCORE-AVX2
	MSVC /arch:AVX2
	NVHPC -tp=haswell
	DEFAULT -march=core-avx2 -mtune=core-avx2 -mrtm
	)
	ENDIF()

.

[dalg24]

If this PR is about fixing a bug (_mm256_mask{load,store}_epi64 intrinsics expecting pointers to long long and us passing pointers to value_type) then drop the unrelated "convenience" changes.

[dalg24]

Why did you add that assertion?

[masterleinad]

As a minimal safeguard that the reinterpret_cast is reasonable.

[masterleinad]

If this PR is about fixing a bug (_mm256_mask{load,store}_epi64 intrinsics expecting pointers to long long and us passing pointers to value_type) then drop the unrelated "convenience" changes.

Here you go!

[dalg24]

Why was this bug not caught before? Is there a gap in the testing (not necessarily the CI, presumably someone built on Broadwell before and it did not give a compile error)?

[ibaned]

I have done Broadwell builds with no errors before this fix... I'm not sure if it is compiler or arch specific...

[masterleinad]

I have done Broadwell builds with no errors before this fix... I'm not sure if it is compiler or arch specific...

My testing shows that this would only have worked on Linux 32-bit architectures.

[ibaned]

maybe I never did un-masked loads of int64 vectors? Could be a gap in unit testing? Thanks for catching of course!

[dalg24]

I have done Broadwell builds with no errors before this fix... I'm not sure if it is compiler or arch specific...

My testing shows that this would only have worked on Linux 32-bit architectures.

@masterleinad meaning with the current develop you got a compile error? I am trying to determine whether we need to add unit tests (excluding the whether we need some more CI/nightly build)

[masterleinad]

@masterleinad meaning with the current develop you got a compile error? I am trying to determine whether we need to add unit tests (excluding the whether we need some more CI/nightly build)

You see this with current develop and current tests but we don't test any host architectures in our GitHub or Jenkins CI (that's part of the reason why I am proposing to let Kokkos_ARCH_NATIVE enable SIMD intrinsic).

[dalg24]

You see this with current develop and current tests

OK

but we don't test any host architectures in our GitHub or Jenkins CI (that's part of the reason why I am proposing to let Kokkos_ARCH_NATIVE enable SIMD intrinsic).

Understood but this must be handled elsewhere.

[dalg24]

this must be handled elsewhere.

To elaborate on this a bit more, whether to enable based on compiler defines rather than user-provided configure options is a question that we need to examine with care and it is beyond the scope of this PR. I am not opposed to it, it is actually in my backlog, I was considering getting rid of that macro along with others such as KOKKOS_ARCH_AMDAVX, KOKKOS_ARCH_AMD_AVX2, KOKKOS_ARCH_AVX512MIC, or KOKKOS_ARCH_AVX512XEON.

[dalg24]

Retest this please