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

[Libomptarget] Remove unnecessary CMake definition of endiannness #77205

Merged
merged 1 commit into from Jan 8, 2024

Conversation

jhuber6
Copy link
Contributor

@jhuber6 jhuber6 commented Jan 6, 2024

Summary:
This is needed for some definition in hsa.h that requires this to be
set for some architectures when it fails at autodetection. We only
really build libomptarget with gcc and clang which already provide
their own way of detecting this. Remove the unnecessary define and move
it into the source.

Summary:
This is needed for some definition in `hsa.h` that requires this to be
set for some architectures when it fails at autodetection. We only
really build `libomptarget` with `gcc` and `clang` which already provide
their own way of detecting this. Remove the unnecessary define and move
it into the source.
@llvmbot
Copy link
Collaborator

llvmbot commented Jan 6, 2024

@llvm/pr-subscribers-backend-amdgpu

Author: Joseph Huber (jhuber6)

Changes

Summary:
This is needed for some definition in hsa.h that requires this to be
set for some architectures when it fails at autodetection. We only
really build libomptarget with gcc and clang which already provide
their own way of detecting this. Remove the unnecessary define and move
it into the source.


Full diff: https://github.com/llvm/llvm-project/pull/77205.diff

2 Files Affected:

  • (modified) openmp/libomptarget/plugins-nextgen/amdgpu/CMakeLists.txt (-8)
  • (modified) openmp/libomptarget/plugins-nextgen/amdgpu/src/rtl.cpp (+12)
diff --git a/openmp/libomptarget/plugins-nextgen/amdgpu/CMakeLists.txt b/openmp/libomptarget/plugins-nextgen/amdgpu/CMakeLists.txt
index bbf8c0a50f85c4..68ce63467a6c81 100644
--- a/openmp/libomptarget/plugins-nextgen/amdgpu/CMakeLists.txt
+++ b/openmp/libomptarget/plugins-nextgen/amdgpu/CMakeLists.txt
@@ -35,14 +35,6 @@ add_definitions(-DTARGET_NAME=AMDGPU)
 # requires changing the original plugins.
 add_definitions(-DDEBUG_PREFIX="TARGET AMDGPU RTL")
 
-if(CMAKE_SYSTEM_PROCESSOR MATCHES "(ppc64le)|(aarch64)$")
-   add_definitions(-DLITTLEENDIAN_CPU=1)
-endif()
-
-if(CMAKE_BUILD_TYPE MATCHES Debug)
-  add_definitions(-DDEBUG)
-endif()
-
 set(LIBOMPTARGET_DLOPEN_LIBHSA OFF)
 option(LIBOMPTARGET_FORCE_DLOPEN_LIBHSA "Build with dlopened libhsa" ${LIBOMPTARGET_DLOPEN_LIBHSA})
 
diff --git a/openmp/libomptarget/plugins-nextgen/amdgpu/src/rtl.cpp b/openmp/libomptarget/plugins-nextgen/amdgpu/src/rtl.cpp
index 0411c670133422..0eab5bc50c4aae 100644
--- a/openmp/libomptarget/plugins-nextgen/amdgpu/src/rtl.cpp
+++ b/openmp/libomptarget/plugins-nextgen/amdgpu/src/rtl.cpp
@@ -43,6 +43,18 @@
 #include "llvm/Support/Program.h"
 #include "llvm/Support/raw_ostream.h"
 
+#if !defined(__BYTE_ORDER__) || !defined(__ORDER_LITTLE_ENDIAN__) ||           \
+    !defined(__ORDER_BIG_ENDIAN__)
+#error "Missing preprocessor definitions for endianness detection."
+#endif
+
+// The HSA headers require these definitions.
+#if defined(__BYTE_ORDER__) && (__BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__)
+#define LITTLEENDIAN_CPU
+#elif defined(__BYTE_ORDER__) && (__BYTE_ORDER__ == __ORDER_BIG_ENDIAN__)
+#define BIGENDIAN_CPU
+#endif
+
 #if defined(__has_include)
 #if __has_include("hsa/hsa.h")
 #include "hsa/hsa.h"

@jhuber6 jhuber6 merged commit e7655ad into llvm:main Jan 8, 2024
6 checks passed
justinfargnoli pushed a commit to justinfargnoli/llvm-project that referenced this pull request Jan 28, 2024
…vm#77205)

Summary:
This is needed for some definition in `hsa.h` that requires this to be
set for some architectures when it fails at autodetection. We only
really build `libomptarget` with `gcc` and `clang` which already provide
their own way of detecting this. Remove the unnecessary define and move
it into the source.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants