[Offload] Don't check in generated files #141982
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@llvm/pr-subscribers-offload Author: Callum Fare (callumfare) ChangesPreviously we decided to check in files that we generate with tablegen. The justification at the time was that it helped reviewers unfamiliar with This changes our use of tablegen to be more conventional. Where possible, files are still clang-formatted, but this is no longer a hard requirement. Because Patch is 106.16 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/141982.diff 11 Files Affected:
diff --git a/offload/CMakeLists.txt b/offload/CMakeLists.txt
index c7cafd105f52a..09eae0f5d3aea 100644
--- a/offload/CMakeLists.txt
+++ b/offload/CMakeLists.txt
@@ -363,6 +363,8 @@ set(LIBOMPTARGET_LLVM_LIBRARY_DIR "${LLVM_LIBRARY_DIR}" CACHE STRING
set(LIBOMPTARGET_LLVM_LIBRARY_INTDIR "${LIBOMPTARGET_INTDIR}" CACHE STRING
"Path to folder where intermediate libraries will be output")
+add_subdirectory(tools/offload-tblgen)
+
# Build offloading plugins and device RTLs if they are available.
add_subdirectory(plugins-nextgen)
add_subdirectory(DeviceRTL)
@@ -371,7 +373,6 @@ add_subdirectory(tools)
# Build target agnostic offloading library.
add_subdirectory(libomptarget)
-add_subdirectory(tools/offload-tblgen)
add_subdirectory(liboffload)
# Add tests.
diff --git a/offload/include/Shared/OffloadErrcodes.inc b/offload/include/Shared/OffloadErrcodes.inc
deleted file mode 100644
index 130e553aa70a4..0000000000000
--- a/offload/include/Shared/OffloadErrcodes.inc
+++ /dev/null
@@ -1,51 +0,0 @@
-//===- Auto-generated file, part of the LLVM/Offload project --------------===//
-//
-// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
-// See https://llvm.org/LICENSE.txt for license information.
-// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-//
-//===----------------------------------------------------------------------===//
-
-#ifndef OFFLOAD_ERRC
-#error Please define the macro OFFLOAD_ERRCODE(Name, Desc, Value)
-#endif
-
-// Error codes are shared between PluginInterface and liboffload.
-// To add new error codes, add them to offload/liboffload/API/Common.td and run
-// the GenerateOffload target.
-
-OFFLOAD_ERRC(SUCCESS, "success", 0)
-OFFLOAD_ERRC(UNKNOWN, "unknown or internal error", 1)
-OFFLOAD_ERRC(HOST_IO, "I/O error on host", 2)
-OFFLOAD_ERRC(INVALID_BINARY, "a provided binary image is malformed", 3)
-OFFLOAD_ERRC(INVALID_NULL_POINTER,
- "a pointer argument is null when it should not be", 4)
-OFFLOAD_ERRC(INVALID_ARGUMENT, "an argument is invalid", 5)
-OFFLOAD_ERRC(NOT_FOUND, "requested object was not found in the binary image", 6)
-OFFLOAD_ERRC(OUT_OF_RESOURCES, "out of resources", 7)
-OFFLOAD_ERRC(
- INVALID_SIZE,
- "invalid size or dimensions (e.g., must not be zero, or is out of bounds)",
- 8)
-OFFLOAD_ERRC(INVALID_ENUMERATION, "enumerator argument is not valid", 9)
-OFFLOAD_ERRC(HOST_TOOL_NOT_FOUND,
- "a required binary (linker, etc.) was not found on the host", 10)
-OFFLOAD_ERRC(INVALID_VALUE, "invalid value", 11)
-OFFLOAD_ERRC(UNIMPLEMENTED,
- "generic error code for features currently unimplemented by the "
- "device/backend",
- 12)
-OFFLOAD_ERRC(
- UNSUPPORTED,
- "generic error code for features unsupported by the device/backend", 13)
-OFFLOAD_ERRC(ASSEMBLE_FAILURE,
- "assembler failure while processing binary image", 14)
-OFFLOAD_ERRC(LINK_FAILURE, "linker failure while processing binary image", 15)
-OFFLOAD_ERRC(BACKEND_FAILURE,
- "the plugin backend is in an invalid or unsupported state", 16)
-OFFLOAD_ERRC(INVALID_NULL_HANDLE,
- "a handle argument is null when it should not be", 17)
-OFFLOAD_ERRC(INVALID_PLATFORM, "invalid platform", 18)
-OFFLOAD_ERRC(INVALID_DEVICE, "invalid device", 19)
-OFFLOAD_ERRC(INVALID_QUEUE, "invalid queue", 20)
-OFFLOAD_ERRC(INVALID_EVENT, "invalid event", 21)
diff --git a/offload/liboffload/API/CMakeLists.txt b/offload/liboffload/API/CMakeLists.txt
index 5f8d1435d141f..cf6e132aa57a9 100644
--- a/offload/liboffload/API/CMakeLists.txt
+++ b/offload/liboffload/API/CMakeLists.txt
@@ -1,29 +1,46 @@
-# The OffloadGenerate target is used to regenerate the generated files in the
-# include directory. These files are checked in with the rest of the source,
-# therefore it is only needed when making changes to the API.
+# We want to clang-format the generated files if possible, since OffloadAPI.h is
+# the main public header for liboffload. Generate them in a temporary location,
+# then clang-format and copy them to the proper location. If clang-format is
+# missing just copy them.
+# Ideally we'd just clang-format them in place and avoid the copy but cmake
+# gets confused about the same path being a byproduct of two custom commands.
-find_program(CLANG_FORMAT clang-format PATHS ${LLVM_TOOLS_BINARY_DIR} NO_DEFAULT_PATH)
-if (CLANG_FORMAT)
- set(LLVM_TARGET_DEFINITIONS ${CMAKE_CURRENT_SOURCE_DIR}/OffloadAPI.td)
+set(LLVM_TARGET_DEFINITIONS ${CMAKE_CURRENT_SOURCE_DIR}/OffloadAPI.td)
+set(files_to_copy "")
- tablegen(OFFLOAD OffloadAPI.h -gen-api)
- tablegen(OFFLOAD OffloadEntryPoints.inc -gen-entry-points)
- tablegen(OFFLOAD OffloadFuncs.inc -gen-func-names)
- tablegen(OFFLOAD OffloadImplFuncDecls.inc -gen-impl-func-decls)
- tablegen(OFFLOAD OffloadPrint.hpp -gen-print-header)
- tablegen(OFFLOAD OffloadErrcodes.inc -gen-errcodes)
+macro(offload_tablegen file)
+ tablegen(OFFLOAD generated/${file}.gen ${ARGN})
+ list(APPEND files_to_copy ${file})
+endmacro()
- set(FILES_TO_COPY "OffloadAPI.h;OffloadEntryPoints.inc;OffloadFuncs.inc;OffloadImplFuncDecls.inc;OffloadPrint.hpp")
- set(GEN_DIR ${CMAKE_CURRENT_SOURCE_DIR}/../include/generated)
- add_public_tablegen_target(OffloadGenerate)
- add_custom_command(TARGET OffloadGenerate POST_BUILD COMMAND ${CLANG_FORMAT}
- -i ${TABLEGEN_OUTPUT})
- add_custom_command(TARGET OffloadGenerate POST_BUILD COMMAND ${CMAKE_COMMAND}
- -E copy_if_different ${FILES_TO_COPY} ${GEN_DIR})
- add_custom_command(TARGET OffloadGenerate POST_BUILD COMMAND ${CMAKE_COMMAND}
- -E copy_if_different OffloadErrcodes.inc "${LIBOMPTARGET_INCLUDE_DIR}/Shared/OffloadErrcodes.inc")
+offload_tablegen(OffloadAPI.h -gen-api)
+offload_tablegen(OffloadEntryPoints.inc -gen-entry-points)
+offload_tablegen(OffloadFuncs.inc -gen-func-names)
+offload_tablegen(OffloadImplFuncDecls.inc -gen-impl-func-decls)
+offload_tablegen(OffloadPrint.hpp -gen-print-header)
+
+add_public_tablegen_target(OffloadGenerate)
+
+add_custom_target(OffloadAPI DEPENDS OffloadGenerate)
+find_program(clang_format clang-format PATHS ${LLVM_TOOLS_BINARY_DIR} NO_DEFAULT_PATH)
+if (clang_format)
+ foreach(file IN LISTS files_to_copy)
+ add_custom_command(
+ OUTPUT ${file}
+ COMMAND ${clang_format} -i generated/${file}.gen
+ COMMAND ${CMAKE_COMMAND} -E copy_if_different generated/${file}.gen ${CMAKE_CURRENT_BINARY_DIR}/${file}
+ DEPENDS generated/${file}.gen
+ )
+ add_custom_target(OffloadAPI.${file} DEPENDS ${file})
+ add_dependencies(OffloadAPI OffloadAPI.${file})
+ endforeach()
else()
- message(WARNING "clang-format was not found, so the OffloadGenerate target\
- will not be available. Offload will still build, but you will not be\
- able to make changes to the API.")
+ message(WARNING "clang-format not found, the generated Offload API headers will not be formatted")
+ foreach(file IN LISTS files_to_copy)
+ add_custom_command(
+ OUTPUT ${file}
+ COMMAND ${CMAKE_COMMAND} -E copy_if_different generated/${file}.gen ${CMAKE_CURRENT_BINARY_DIR}/${file}
+ DEPENDS generated/${file}.gen
+ )
+ endforeach()
endif()
diff --git a/offload/liboffload/CMakeLists.txt b/offload/liboffload/CMakeLists.txt
index 1b098bc01e218..e9edf66b9a669 100644
--- a/offload/liboffload/CMakeLists.txt
+++ b/offload/liboffload/CMakeLists.txt
@@ -8,6 +8,9 @@ add_llvm_library(
LINK_COMPONENTS
FrontendOpenMP
Support
+
+ DEPENDS
+ OffloadAPI
)
foreach(plugin IN LISTS LIBOMPTARGET_PLUGINS_TO_BUILD)
@@ -19,9 +22,9 @@ if(LLVM_HAVE_LINK_VERSION_SCRIPT)
endif()
target_include_directories(LLVMOffload PUBLIC
+ ${CMAKE_CURRENT_BINARY_DIR}/API
${CMAKE_CURRENT_BINARY_DIR}/../include
${CMAKE_CURRENT_SOURCE_DIR}/include
- ${CMAKE_CURRENT_SOURCE_DIR}/include/generated
${CMAKE_CURRENT_SOURCE_DIR}/../include
${CMAKE_CURRENT_SOURCE_DIR}/../plugins-nextgen/common/include)
@@ -39,5 +42,5 @@ set_target_properties(LLVMOffload PROPERTIES
BUILD_RPATH "$ORIGIN:${CMAKE_CURRENT_BINARY_DIR}/..")
install(TARGETS LLVMOffload LIBRARY COMPONENT LLVMOffload DESTINATION "${OFFLOAD_INSTALL_LIBDIR}")
-install(FILES ${CMAKE_CURRENT_SOURCE_DIR}/include/generated/OffloadAPI.h DESTINATION ${CMAKE_INSTALL_PREFIX}/include/offload)
-install(FILES ${CMAKE_CURRENT_SOURCE_DIR}/include/generated/OffloadPrint.hpp DESTINATION ${CMAKE_INSTALL_PREFIX}/include/offload)
+install(FILES ${CMAKE_CURRENT_BINARY_DIR}/API/OffloadAPI.h DESTINATION ${CMAKE_INSTALL_PREFIX}/include/offload)
+install(FILES ${CMAKE_CURRENT_BINARY_DIR}/API/OffloadPrint.hpp DESTINATION ${CMAKE_INSTALL_PREFIX}/include/offload)
diff --git a/offload/liboffload/include/generated/OffloadAPI.h b/offload/liboffload/include/generated/OffloadAPI.h
deleted file mode 100644
index a1d7540519e32..0000000000000
--- a/offload/liboffload/include/generated/OffloadAPI.h
+++ /dev/null
@@ -1,1021 +0,0 @@
-//===- Auto-generated file, part of the LLVM/Offload project --------------===//
-//
-// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
-// See https://llvm.org/LICENSE.txt for license information.
-// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-//
-//===----------------------------------------------------------------------===//
-
-// Auto-generated file, do not manually edit.
-
-#pragma once
-
-#include <stddef.h>
-#include <stdint.h>
-
-#if defined(__cplusplus)
-extern "C" {
-#endif
-
-///////////////////////////////////////////////////////////////////////////////
-/// @brief Defines Return/Error codes
-typedef enum ol_errc_t {
- /// success
- OL_ERRC_SUCCESS = 0,
- /// unknown or internal error
- OL_ERRC_UNKNOWN = 1,
- /// I/O error on host
- OL_ERRC_HOST_IO = 2,
- /// a provided binary image is malformed
- OL_ERRC_INVALID_BINARY = 3,
- /// a pointer argument is null when it should not be
- OL_ERRC_INVALID_NULL_POINTER = 4,
- /// an argument is invalid
- OL_ERRC_INVALID_ARGUMENT = 5,
- /// requested object was not found in the binary image
- OL_ERRC_NOT_FOUND = 6,
- /// out of resources
- OL_ERRC_OUT_OF_RESOURCES = 7,
- /// invalid size or dimensions (e.g., must not be zero, or is out of bounds)
- OL_ERRC_INVALID_SIZE = 8,
- /// enumerator argument is not valid
- OL_ERRC_INVALID_ENUMERATION = 9,
- /// a required binary (linker, etc.) was not found on the host
- OL_ERRC_HOST_TOOL_NOT_FOUND = 10,
- /// invalid value
- OL_ERRC_INVALID_VALUE = 11,
- /// generic error code for features currently unimplemented by the
- /// device/backend
- OL_ERRC_UNIMPLEMENTED = 12,
- /// generic error code for features unsupported by the device/backend
- OL_ERRC_UNSUPPORTED = 13,
- /// assembler failure while processing binary image
- OL_ERRC_ASSEMBLE_FAILURE = 14,
- /// linker failure while processing binary image
- OL_ERRC_LINK_FAILURE = 15,
- /// the plugin backend is in an invalid or unsupported state
- OL_ERRC_BACKEND_FAILURE = 16,
- /// a handle argument is null when it should not be
- OL_ERRC_INVALID_NULL_HANDLE = 17,
- /// invalid platform
- OL_ERRC_INVALID_PLATFORM = 18,
- /// invalid device
- OL_ERRC_INVALID_DEVICE = 19,
- /// invalid queue
- OL_ERRC_INVALID_QUEUE = 20,
- /// invalid event
- OL_ERRC_INVALID_EVENT = 21,
- /// @cond
- OL_ERRC_FORCE_UINT32 = 0x7fffffff
- /// @endcond
-
-} ol_errc_t;
-
-///////////////////////////////////////////////////////////////////////////////
-#ifndef OL_VERSION_MAJOR
-/// @brief Major version of the Offload API
-#define OL_VERSION_MAJOR 0
-#endif // OL_VERSION_MAJOR
-
-///////////////////////////////////////////////////////////////////////////////
-#ifndef OL_VERSION_MINOR
-/// @brief Minor version of the Offload API
-#define OL_VERSION_MINOR 0
-#endif // OL_VERSION_MINOR
-
-///////////////////////////////////////////////////////////////////////////////
-#ifndef OL_VERSION_PATCH
-/// @brief Patch version of the Offload API
-#define OL_VERSION_PATCH 1
-#endif // OL_VERSION_PATCH
-
-///////////////////////////////////////////////////////////////////////////////
-#ifndef OL_APICALL
-#if defined(_WIN32)
-/// @brief Calling convention for all API functions
-#define OL_APICALL __cdecl
-#else
-#define OL_APICALL
-#endif // defined(_WIN32)
-#endif // OL_APICALL
-
-///////////////////////////////////////////////////////////////////////////////
-#ifndef OL_APIEXPORT
-#if defined(_WIN32)
-/// @brief Microsoft-specific dllexport storage-class attribute
-#define OL_APIEXPORT __declspec(dllexport)
-#else
-#define OL_APIEXPORT
-#endif // defined(_WIN32)
-#endif // OL_APIEXPORT
-
-///////////////////////////////////////////////////////////////////////////////
-#ifndef OL_DLLEXPORT
-#if defined(_WIN32)
-/// @brief Microsoft-specific dllexport storage-class attribute
-#define OL_DLLEXPORT __declspec(dllexport)
-#endif // defined(_WIN32)
-#endif // OL_DLLEXPORT
-
-///////////////////////////////////////////////////////////////////////////////
-#ifndef OL_DLLEXPORT
-#if __GNUC__ >= 4
-/// @brief GCC-specific dllexport storage-class attribute
-#define OL_DLLEXPORT __attribute__((visibility("default")))
-#else
-#define OL_DLLEXPORT
-#endif // __GNUC__ >= 4
-#endif // OL_DLLEXPORT
-
-///////////////////////////////////////////////////////////////////////////////
-/// @brief Handle of a platform instance
-typedef struct ol_platform_impl_t *ol_platform_handle_t;
-
-///////////////////////////////////////////////////////////////////////////////
-/// @brief Handle of platform's device object
-typedef struct ol_device_impl_t *ol_device_handle_t;
-
-///////////////////////////////////////////////////////////////////////////////
-/// @brief Handle of context object
-typedef struct ol_context_impl_t *ol_context_handle_t;
-
-///////////////////////////////////////////////////////////////////////////////
-/// @brief Handle of queue object
-typedef struct ol_queue_impl_t *ol_queue_handle_t;
-
-///////////////////////////////////////////////////////////////////////////////
-/// @brief Handle of event object
-typedef struct ol_event_impl_t *ol_event_handle_t;
-
-///////////////////////////////////////////////////////////////////////////////
-/// @brief Handle of program object
-typedef struct ol_program_impl_t *ol_program_handle_t;
-
-///////////////////////////////////////////////////////////////////////////////
-/// @brief Handle of kernel object
-typedef void *ol_kernel_handle_t;
-
-///////////////////////////////////////////////////////////////////////////////
-/// @brief Details of the error condition returned by an API call
-typedef struct ol_error_struct_t {
- ol_errc_t Code; /// The error code
- const char *Details; /// String containing error details
-} ol_error_struct_t;
-
-///////////////////////////////////////////////////////////////////////////////
-/// @brief Result type returned by all entry points.
-typedef const ol_error_struct_t *ol_result_t;
-
-///////////////////////////////////////////////////////////////////////////////
-#ifndef OL_SUCCESS
-/// @brief Success condition
-#define OL_SUCCESS NULL
-#endif // OL_SUCCESS
-
-///////////////////////////////////////////////////////////////////////////////
-/// @brief Code location information that can optionally be associated with an
-/// API call
-typedef struct ol_code_location_t {
- const char *FunctionName; /// Function name
- const char *SourceFile; /// Source code file
- uint32_t LineNumber; /// Source code line number
- uint32_t ColumnNumber; /// Source code column number
-} ol_code_location_t;
-
-///////////////////////////////////////////////////////////////////////////////
-/// @brief Perform initialization of the Offload library and plugins
-///
-/// @details
-/// - This must be the first API call made by a user of the Offload library
-/// - Each call will increment an internal reference count that is
-/// decremented by `olShutDown`
-///
-/// @returns
-/// - ::OL_RESULT_SUCCESS
-/// - ::OL_ERRC_UNINITIALIZED
-/// - ::OL_ERRC_DEVICE_LOST
-/// - ::OL_ERRC_INVALID_NULL_HANDLE
-/// - ::OL_ERRC_INVALID_NULL_POINTER
-OL_APIEXPORT ol_result_t OL_APICALL olInit();
-
-///////////////////////////////////////////////////////////////////////////////
-/// @brief Release the resources in use by Offload
-///
-/// @details
-/// - This decrements an internal reference count. When this reaches 0, all
-/// resources will be released
-/// - Subsequent API calls made after this are not valid
-///
-/// @returns
-/// - ::OL_RESULT_SUCCESS
-/// - ::OL_ERRC_UNINITIALIZED
-/// - ::OL_ERRC_DEVICE_LOST
-/// - ::OL_ERRC_INVALID_NULL_HANDLE
-/// - ::OL_ERRC_INVALID_NULL_POINTER
-OL_APIEXPORT ol_result_t OL_APICALL olShutDown();
-
-///////////////////////////////////////////////////////////////////////////////
-/// @brief Supported platform info.
-typedef enum ol_platform_info_t {
- /// [char[]] The string denoting name of the platform. The size of the info
- /// needs to be dynamically queried.
- OL_PLATFORM_INFO_NAME = 0,
- /// [char[]] The string denoting name of the vendor of the platform. The size
- /// of the info needs to be dynamically queried.
- OL_PLATFORM_INFO_VENDOR_NAME = 1,
- /// [char[]] The string denoting the version of the platform. The size of the
- /// info needs to be dynamically queried.
- OL_PLATFORM_INFO_VERSION = 2,
- /// [ol_platform_backend_t] The native backend of the platform.
- OL_PLATFORM_INFO_BACKEND = 3,
- /// @cond
- OL_PLATFORM_INFO_FORCE_UINT32 = 0x7fffffff
- /// @endcond
-
-} ol_platform_info_t;
-
-///////////////////////////////////////////////////////////////////////////////
-/// @brief Identifies the native backend of the platform.
-typedef enum ol_platform_backend_t {
- /// The backend is not recognized
- OL_PLATFORM_BACKEND_UNKNOWN = 0,
- /// The backend is CUDA
- OL_PLATFORM_BACKEND_CUDA = 1,
- /// The backend is AMDGPU
- OL_PLATFORM_BACKEND_AMDGPU = 2,
- /// The backend is the host
- OL_PLATFORM_BACKEND_HOST = 3,
- /// @cond
- OL_PLATFORM_BACKEND_FORCE_UINT32 = 0x7fffffff
- /// @endcond
-
-} ol_platform_backend_t;
-
-///////////////////////////////////////////////////////////////////////////////
-/// @brief Queries the given property of the platform.
-///
-/// @details
-/// - `olGetPlatformInfoSize` can be used to query the storage size required
-/// for the given query.
-///
-/// @returns
-/// - ::OL_RESULT_SUCCESS
-/// - ::OL_ERRC_UNINITIALIZED
-/// - ::OL_ERRC_DEVICE_LOST
-/// - ::OL_ERRC_UNSUPPORTED_ENUMERATION
-/// + If `PropName` is not supported by the platform.
-/// - ::OL_ERRC_INVALID_SIZE
-/// + `PropSize == 0`
-/// + If `PropSize` is less than the real number of bytes needed to
-/// return the info.
-/// - ::OL_ERRC_INVALID_PLATFORM
-/// - ::OL_ERRC_INVALID_NULL_HANDLE
-/// + `NULL == Platform`
-/// - ::OL_ERRC_INVALID_NULL_POINTER
-/// + `NULL == PropValue`
-OL_APIEXPORT ol_result_t OL_APICALL olGetPlatformInfo(
- // [in] handle of the platform
- ol_platform_handle_t Platform,
- // [in] type of the info to retrieve
- ol_platform_info_t PropName,
- // [in] the number of bytes pointed to by pPlatformInfo.
- size_t PropSize,
- // [out] array of bytes holding the info. If Size is not equal to or greater
- // to the real number of bytes needed to return the info then the
- // OL_ERRC_INVALID_SIZE error is returned and pPlatformInfo is not used.
- void *PropValue);
-
-///////////////////////////////////////////////////////////////////////////////
-/// @brief Returns the storage size of the given platform query.
-///
-/// @details
-///
-/// @returns
-/// - ::OL_RESULT_SUCCESS
-/// - ::OL_ERRC_UNINITIALIZED
-/// - ::OL_ERRC_DEVICE_LOST
-/// - ::OL_ERRC_UNSUPPORTED_ENUMERATION
-/// + If `PropName` is not supported by the platform.
-/// - ::OL_ERRC_INVALID_PLATFORM
-/// - ::OL_ERRC_INVALID_NULL_HANDLE
-/// + `NULL == Platform`
-/// - ::OL_ERRC_INVALID_NULL_POINTER
-/// + `NULL == PropSizeRet`
-OL_APIEXPORT ol_result_t OL_APICALL olGetPlatformInfoSize(
- // [in] handle of the platform
- ol_platfor...
[truncated]
|
jhuber6
reviewed
May 29, 2025
| @@ -19,9 +22,9 @@ if(LLVM_HAVE_LINK_VERSION_SCRIPT) | |||
| endif() | |||
|
|
|||
| target_include_directories(LLVMOffload PUBLIC | |||
| ${CMAKE_CURRENT_BINARY_DIR}/API | |||
There was a problem hiding this comment.
Does it still find the error codes? Those were in shared/ because it needs to be used by the plugins as well.
There was a problem hiding this comment.
Yeah it seems to work fine, the ${CMAKE_CURRENT_BINARY_DIR}/../include line below means it finds Shared/OffloadErrcodes.inc
There was a problem hiding this comment.
Although I do wonder if it should be getting the dependency on the shared header from the plugins themselves rather than libomptarget
There was a problem hiding this comment.
There was a missing dependency between liboffload and the LibomptargetOffloadErrcodes target, I've fixed that now
callumfare
force-pushed
the
offload_on_demand_tablegen
branch
from
7621f98 to
fb1b3cb
Compare
jhuber6
approved these changes
May 29, 2025
callumfare
force-pushed
the
offload_on_demand_tablegen
branch
from
fb1b3cb to
f82dd68
Compare
callumfare
force-pushed
the
offload_on_demand_tablegen
branch
from
f82dd68 to
c18f3a5
Compare
|
@jhuber6 Could you merge this if you get a chance? I'm still waiting on my commit access being approved |
jhuber6
reviewed
Jun 3, 2025
| @@ -32,6 +32,8 @@ function(add_target_library target_name lib_name) | |||
|
|
|||
| NO_INSTALL_RPATH | |||
| BUILDTREE_ONLY | |||
|
|
|||
| DEPENDS LibomptargetOffloadErrcodes | |||
There was a problem hiding this comment.
We have plugins depending on something from libomptarget? That's backwards.
There was a problem hiding this comment.
Yeah this seems weird. I think the OffloadErrcodes.inc header should be part of the plugin interface rather than part of libomptarget. I'm not sure how include/Shared differs from the headers in plugins-nextgen/common/include but it makes sense to consider the error codes part of the plugin API rather than belonging to libomptarget.
There was a problem hiding this comment.
It's just a shared include location, but we could make in an artifact from building the plugins if needed.
There was a problem hiding this comment.
I've moved it to be part of the plugin interface and tidied up the dependencies, I think it makes a lot more sense now
jhuber6
approved these changes
Jun 3, 2025
searlmc1
pushed a commit
to ROCm/llvm-project
that referenced
this pull request
Jun 4, 2025
This reverts commit b78bc35.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Previously we decided to check in files that we generate with tablegen. The justification at the time was that it helped reviewers unfamiliar with
offload-tblgensee the actual changes to the headers in PRs. After trying it for a while, it's ended up causing some headaches and is also not how tablegen is used elsewhere in LLVM.This changes our use of tablegen to be more conventional. Where possible, files are still clang-formatted, but this is no longer a hard requirement. Because
OffloadErrcodes.incis shared with libomptarget it now gets generated in a more appropriate place.