-
Notifications
You must be signed in to change notification settings - Fork 14.3k
[Libomptarget] Consolidate CPU offloading into 'host' directory #86014
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
Conversation
Summary: All of these CPU targets use the same underlying implementation. We should consolidate them into a single target to make it easier to update this to a static library based approach. I have decided to call this the 'host' target so it can be given a single name. We still only build these if the system processor matches and we are on Linux.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LG
"aarch64-unknown-linux-gnu" "aarch64-unknown-linux-gnu-LTO") | ||
set(LIBOMPTARGET_SYSTEM_TARGETS "${LIBOMPTARGET_SYSTEM_TARGETS}" PARENT_SCOPE) | ||
elseif(CMAKE_SYSTEM_PROCESSOR MATCHES "s390x$") | ||
target_compile_definitions(omptarget.rtl.${machine} TARGET_ELF_ID=EM_S390) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's a missing "PRIVATE" on this line, which caused the build to break on SystemZ. Fixed in cb07194.
…#86014) Summary: All of these CPU targets use the same underlying implementation. We should consolidate them into a single target to make it easier to update this to a static library based approach. I have decided to call this the 'host' target so it can be given a single name. We still only build these if the system processor matches and we are on Linux.
target_compile_definitions(omptarget.rtl.${machine} PRIVATE TARGET_ELF_ID=EM_PPC64) | ||
target_compile_definitions(omptarget.rtl.${machine} PRIVATE | ||
LIBOMPTARGET_NEXTGEN_GENERIC_PLUGIN_TRIPLE="powerpc64-ibm-linux-gnu") | ||
list(APPEND LIBOMPTARGET_SYSTEM_TARGETS | ||
"powerpc64-ibm-linux-gnu" "powerpc64-ibm-linux-gnu-LTO") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is incorrect, because it is specifying a big-endian triple (powerpc64-ibm-linux-gnu) when the host is little-endian (ppc64le). The ppc64le and ppc64 $CMAKE_SYSTEM_PROCESSOR values need to be handled separately.
cc @tuliom
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@tstellar Agreed. It's worth mentioning that using powerpc64le
in triples is more frequent than ppc64le
.
This is the first time I see -ibm
being used for a ppc64*-linux
system.
Is the usage of -ibm
really necessary? And what it means?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I made #88773. Using the vendor field doesn't really do anything for these triples. It's the same as x86_64-pc-linux-gnu
which doesn't make a difference except for runtime directory stuff.
Summary:
All of these CPU targets use the same underlying implementation. We
should consolidate them into a single target to make it easier to update
this to a static library based approach. I have decided to call this the
'host' target so it can be given a single name. We still only build
these if the system processor matches and we are on Linux.