Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Check for multiple host architectures in CMake #4996

Merged
merged 2 commits into from
May 9, 2022

Conversation

masterleinad
Copy link
Contributor

Fixes #4964. For Makefile.kokkos we already have

kokkos/Makefile.kokkos

Lines 461 to 470 in 14bdcee

# Incompatible flags?
KOKKOS_INTERNAL_USE_ARCH_MULTIHOST := $(strip $(shell echo "$(KOKKOS_INTERNAL_USE_ARCH_SSE42)+$(KOKKOS_INTERNAL_USE_ARCH_AVX)+$(KOKKOS_INTERNAL_USE_ARCH_AVX2)+$(KOKKOS_INTERNAL_USE_ARCH_AVX512MIC)+$(KOKKOS_INTERNAL_USE_ARCH_AVX512XEON)+$(KOKKOS_INTERNAL_USE_ARCH_KNC)+$(KOKKOS_INTERNAL_USE_ARCH_IBM)+$(KOKKOS_INTERNAL_USE_ARCH_ARM)>1") | bc)
KOKKOS_INTERNAL_USE_ARCH_MULTIGPU := $(strip $(shell echo "$(KOKKOS_INTERNAL_USE_ARCH_NVIDIA)>1") | bc)
ifeq ($(KOKKOS_INTERNAL_USE_ARCH_MULTIHOST), 1)
$(error Defined Multiple Host architectures: KOKKOS_ARCH=$(KOKKOS_ARCH) )
endif
ifeq ($(KOKKOS_INTERNAL_USE_ARCH_MULTIGPU), 1)
$(error Defined Multiple GPU architectures: KOKKOS_ARCH=$(KOKKOS_ARCH) )
endif

that detects if multiple architectures are specified.

Copy link
Member

@dalg24 dalg24 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not a big fain of the macro name but I don't have a better idea

@masterleinad
Copy link
Contributor Author

SYCL CI is failing in KokkosCore_PerfTestExec. We see this (spurious) failure from time to time.

@dalg24 dalg24 requested a review from nmm0 May 4, 2022 02:29
Copy link
Contributor

@nmm0 nmm0 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. I wish we wouldn't have to mess up the cache when changing host architectures like on multi arch host systems, but the solution to that would be a bit more complicated.

KOKKOS_ARCH_OPTION(POWER9 HOST "IBM POWER9 CPUs")

SET(HOST_ARCH_ALREADY_SPECIFIED "")
MACRO(CHECK_HOST_ARCH ARCH LABEL)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would rename this to make it clearer that this also declares the option since it's easily missed

Maybe something like DECLARE_AND_CHECK_HOST_ARCH though that's kinda verbose

@dalg24 dalg24 merged commit 1ce24a3 into kokkos:develop May 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants