Skip to content

Commit b78bc35

Browse files
authored
[Offload] Don't check in generated files (#141982)
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-tblgen` see 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.inc` is shared with libomptarget it now gets generated in a more appropriate place.
1 parent e6529dc commit b78bc35

File tree

15 files changed

+70
-2768
lines changed

15 files changed

+70
-2768
lines changed

offload/CMakeLists.txt

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -363,6 +363,8 @@ set(LIBOMPTARGET_LLVM_LIBRARY_DIR "${LLVM_LIBRARY_DIR}" CACHE STRING
363363
set(LIBOMPTARGET_LLVM_LIBRARY_INTDIR "${LIBOMPTARGET_INTDIR}" CACHE STRING
364364
"Path to folder where intermediate libraries will be output")
365365

366+
add_subdirectory(tools/offload-tblgen)
367+
366368
# Build offloading plugins and device RTLs if they are available.
367369
add_subdirectory(plugins-nextgen)
368370
add_subdirectory(DeviceRTL)
@@ -371,7 +373,6 @@ add_subdirectory(tools)
371373
# Build target agnostic offloading library.
372374
add_subdirectory(libomptarget)
373375

374-
add_subdirectory(tools/offload-tblgen)
375376
add_subdirectory(liboffload)
376377

377378
# Add tests.

offload/include/Shared/OffloadErrcodes.inc

Lines changed: 0 additions & 51 deletions
This file was deleted.

offload/liboffload/API/CMakeLists.txt

Lines changed: 41 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1,29 +1,46 @@
1-
# The OffloadGenerate target is used to regenerate the generated files in the
2-
# include directory. These files are checked in with the rest of the source,
3-
# therefore it is only needed when making changes to the API.
1+
# We want to clang-format the generated files if possible, since OffloadAPI.h is
2+
# the main public header for liboffload. Generate them in a temporary location,
3+
# then clang-format and copy them to the proper location. If clang-format is
4+
# missing just copy them.
5+
# Ideally we'd just clang-format them in place and avoid the copy but cmake
6+
# gets confused about the same path being a byproduct of two custom commands.
47

5-
find_program(CLANG_FORMAT clang-format PATHS ${LLVM_TOOLS_BINARY_DIR} NO_DEFAULT_PATH)
6-
if (CLANG_FORMAT)
7-
set(LLVM_TARGET_DEFINITIONS ${CMAKE_CURRENT_SOURCE_DIR}/OffloadAPI.td)
8+
set(LLVM_TARGET_DEFINITIONS ${CMAKE_CURRENT_SOURCE_DIR}/OffloadAPI.td)
9+
set(files_to_copy "")
810

9-
tablegen(OFFLOAD OffloadAPI.h -gen-api)
10-
tablegen(OFFLOAD OffloadEntryPoints.inc -gen-entry-points)
11-
tablegen(OFFLOAD OffloadFuncs.inc -gen-func-names)
12-
tablegen(OFFLOAD OffloadImplFuncDecls.inc -gen-impl-func-decls)
13-
tablegen(OFFLOAD OffloadPrint.hpp -gen-print-header)
14-
tablegen(OFFLOAD OffloadErrcodes.inc -gen-errcodes)
11+
macro(offload_tablegen file)
12+
tablegen(OFFLOAD generated/${file}.gen ${ARGN})
13+
list(APPEND files_to_copy ${file})
14+
endmacro()
1515

16-
set(FILES_TO_COPY "OffloadAPI.h;OffloadEntryPoints.inc;OffloadFuncs.inc;OffloadImplFuncDecls.inc;OffloadPrint.hpp")
17-
set(GEN_DIR ${CMAKE_CURRENT_SOURCE_DIR}/../include/generated)
18-
add_public_tablegen_target(OffloadGenerate)
19-
add_custom_command(TARGET OffloadGenerate POST_BUILD COMMAND ${CLANG_FORMAT}
20-
-i ${TABLEGEN_OUTPUT})
21-
add_custom_command(TARGET OffloadGenerate POST_BUILD COMMAND ${CMAKE_COMMAND}
22-
-E copy_if_different ${FILES_TO_COPY} ${GEN_DIR})
23-
add_custom_command(TARGET OffloadGenerate POST_BUILD COMMAND ${CMAKE_COMMAND}
24-
-E copy_if_different OffloadErrcodes.inc "${LIBOMPTARGET_INCLUDE_DIR}/Shared/OffloadErrcodes.inc")
16+
offload_tablegen(OffloadAPI.h -gen-api)
17+
offload_tablegen(OffloadEntryPoints.inc -gen-entry-points)
18+
offload_tablegen(OffloadFuncs.inc -gen-func-names)
19+
offload_tablegen(OffloadImplFuncDecls.inc -gen-impl-func-decls)
20+
offload_tablegen(OffloadPrint.hpp -gen-print-header)
21+
22+
add_public_tablegen_target(OffloadGenerate)
23+
24+
add_custom_target(OffloadAPI DEPENDS OffloadGenerate)
25+
find_program(clang_format clang-format PATHS ${LLVM_TOOLS_BINARY_DIR} NO_DEFAULT_PATH)
26+
if (clang_format)
27+
foreach(file IN LISTS files_to_copy)
28+
add_custom_command(
29+
OUTPUT ${file}
30+
COMMAND ${clang_format} -i generated/${file}.gen
31+
COMMAND ${CMAKE_COMMAND} -E copy_if_different generated/${file}.gen ${CMAKE_CURRENT_BINARY_DIR}/${file}
32+
DEPENDS generated/${file}.gen
33+
)
34+
add_custom_target(OffloadAPI.${file} DEPENDS ${file})
35+
add_dependencies(OffloadAPI OffloadAPI.${file})
36+
endforeach()
2537
else()
26-
message(WARNING "clang-format was not found, so the OffloadGenerate target\
27-
will not be available. Offload will still build, but you will not be\
28-
able to make changes to the API.")
38+
message(WARNING "clang-format not found, the generated Offload API headers will not be formatted")
39+
foreach(file IN LISTS files_to_copy)
40+
add_custom_command(
41+
OUTPUT ${file}
42+
COMMAND ${CMAKE_COMMAND} -E copy_if_different generated/${file}.gen ${CMAKE_CURRENT_BINARY_DIR}/${file}
43+
DEPENDS generated/${file}.gen
44+
)
45+
endforeach()
2946
endif()

offload/liboffload/CMakeLists.txt

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,10 @@ add_llvm_library(
88
LINK_COMPONENTS
99
FrontendOpenMP
1010
Support
11+
12+
DEPENDS
13+
OffloadAPI
14+
PluginErrcodes
1115
)
1216

1317
foreach(plugin IN LISTS LIBOMPTARGET_PLUGINS_TO_BUILD)
@@ -19,11 +23,13 @@ if(LLVM_HAVE_LINK_VERSION_SCRIPT)
1923
endif()
2024

2125
target_include_directories(LLVMOffload PUBLIC
26+
${CMAKE_CURRENT_BINARY_DIR}/API
2227
${CMAKE_CURRENT_BINARY_DIR}/../include
2328
${CMAKE_CURRENT_SOURCE_DIR}/include
24-
${CMAKE_CURRENT_SOURCE_DIR}/include/generated
2529
${CMAKE_CURRENT_SOURCE_DIR}/../include
26-
${CMAKE_CURRENT_SOURCE_DIR}/../plugins-nextgen/common/include)
30+
${CMAKE_CURRENT_SOURCE_DIR}/../plugins-nextgen/common/include
31+
${CMAKE_CURRENT_BINARY_DIR}/../plugins-nextgen/common/include
32+
)
2733

2834
target_compile_options(LLVMOffload PRIVATE ${offload_compile_flags})
2935
target_link_options(LLVMOffload PRIVATE ${offload_link_flags})
@@ -39,5 +45,5 @@ set_target_properties(LLVMOffload PROPERTIES
3945
BUILD_RPATH "$ORIGIN:${CMAKE_CURRENT_BINARY_DIR}/..")
4046
install(TARGETS LLVMOffload LIBRARY COMPONENT LLVMOffload DESTINATION "${OFFLOAD_INSTALL_LIBDIR}")
4147

42-
install(FILES ${CMAKE_CURRENT_SOURCE_DIR}/include/generated/OffloadAPI.h DESTINATION ${CMAKE_INSTALL_PREFIX}/include/offload)
43-
install(FILES ${CMAKE_CURRENT_SOURCE_DIR}/include/generated/OffloadPrint.hpp DESTINATION ${CMAKE_INSTALL_PREFIX}/include/offload)
48+
install(FILES ${CMAKE_CURRENT_BINARY_DIR}/API/OffloadAPI.h DESTINATION ${CMAKE_INSTALL_PREFIX}/include/offload)
49+
install(FILES ${CMAKE_CURRENT_BINARY_DIR}/API/OffloadPrint.hpp DESTINATION ${CMAKE_INSTALL_PREFIX}/include/offload)

0 commit comments

Comments
 (0)