Skip to content

Commit 1633679

Browse files
AndreyPavlenkoAndreyPavlenko
committed
Apply suggestions from code review
1 parent 50d0ddb commit 1633679

33 files changed

+281
-246
lines changed

‎CMakeLists.txt

+63-110
Original file line numberDiff line numberDiff line change
@@ -18,159 +18,112 @@
1818
cmake_minimum_required(VERSION 3.20)
1919
project(GraphCompiler VERSION "0.1.0" LANGUAGES C CXX)
2020

21-
set(CMAKE_CXX_STANDARD 17)
21+
############################# Cmake options ####################################
22+
set(CMAKE_EXPORT_COMPILE_COMMANDS ON)
2223
set(CMAKE_CXX_STANDARD_REQUIRED ON)
2324
set(CMAKE_CXX_FLAGS_DEBUG "${CMAKE_CXX_FLAGS_DEBUG} -O0")
24-
25-
set(CMAKE_EXPORT_COMPILE_COMMANDS ON)
26-
2725
list(APPEND CMAKE_MODULE_PATH "${PROJECT_SOURCE_DIR}/cmake")
2826

27+
# Silence a false positive GCC -Wunused-but-set-parameter warning in constexpr
28+
# cases, by marking SelectedCase as used. See
29+
# https://gcc.gnu.org/bugzilla/show_bug.cgi?id=85827 for details. The issue is
30+
# fixed in GCC 10.
31+
if(CMAKE_CXX_COMPILER_ID MATCHES "GNU" AND CMAKE_CXX_COMPILER_VERSION VERSION_LESS "10.0")
32+
check_cxx_compiler_flag("-Wno-unused-but-set-parameter" CXX_SUPPORTS_WNO_UNUSED_BUT_SET_PARAMETER)
33+
append_if(CXX_SUPPORTS_WNO_UNUSED_BUT_SET_PARAMETER "-Wno-unused-but-set-parameter" CMAKE_CXX_FLAGS)
34+
endif()
35+
################################################################################
36+
37+
############################ Build options #####################################
2938
option(GC_ENABLE_LEGACY ON)
3039
option(GC_ENABLE_DNNL "Enable the oneDNN library integration" ON)
3140
option(GC_ENABLE_TEST "Build the tests" ON)
3241
option(GC_ENABLE_TEST_DNNL "Build the dnnl tests" ${GC_ENABLE_DNNL})
3342
option(GC_ENABLE_TEST_MLIR "Build the mlir tests" ON)
34-
option(GC_ENABLE_OPT "Build gc-opt" ON)
43+
option(GC_ENABLE_TOOLS "Build the tools" ON)
44+
option(GC_ENABLE_OPT "Build gc-opt" ${GC_ENABLE_TOOLS})
3545
option(GC_ENABLE_GPU "Enable GPU backend" OFF)
3646
option(GC_ENABLE_IMEX "Intel® Extension for MLIR" OFF)
3747
option(GC_ENABLE_BINDINGS_PYTHON "Enable Graph Complier Python Binding" ON)
3848
option(GC_DEV_LINK_LLVM_DYLIB "Link dynamic libraries of LLVM and MLIR. For developers only. Do not use it in packing the library." OFF)
3949

4050
if(GC_ENABLE_LEGACY)
41-
add_subdirectory(legacy/core)
51+
add_subdirectory(legacy/core)
4252
endif()
4353

44-
find_package(MLIR REQUIRED CONFIG)
45-
46-
message(STATUS "Using MLIRConfig.cmake in: ${MLIR_DIR}")
47-
message(STATUS "Using LLVMConfig.cmake in: ${LLVM_DIR}")
48-
49-
set(LLVM_RUNTIME_OUTPUT_INTDIR ${PROJECT_BINARY_DIR}/bin)
50-
set(LLVM_LIBRARY_OUTPUT_INTDIR ${PROJECT_BINARY_DIR}/lib)
51-
set(MLIR_BINARY_DIR ${PROJECT_BINARY_DIR})
52-
53-
list(APPEND CMAKE_MODULE_PATH "${MLIR_CMAKE_DIR}")
54-
list(APPEND CMAKE_MODULE_PATH "${LLVM_CMAKE_DIR}")
55-
56-
include(TableGen)
57-
include(AddLLVM)
58-
include(AddMLIR)
59-
include(HandleLLVMOptions)
60-
6154
if(GC_ENABLE_DNNL)
62-
add_definitions(-DGC_HAS_ONEDNN_DIALECT)
6355
set(GC_ONEDNN_DIALECT_LIB_NAME MLIROneDNNGraph)
64-
endif ()
65-
66-
if(GC_ENABLE_GPU)
67-
add_definitions(-DGC_USE_GPU)
68-
endif ()
69-
70-
if(GC_ENABLE_IMEX)
71-
include(imex)
72-
if(GC_DEV_LINK_LLVM_DYLIB)
73-
message(WARN "GPU backend may not be compatible with dynamic linking to LLVM")
74-
endif()
75-
endif()
76-
77-
if (GC_ENABLE_OPT)
78-
set(GC_OPT_EXE_NAME gc-opt)
79-
endif ()
80-
81-
if(GC_ENABLE_BINDINGS_PYTHON AND NOT MLIR_ENABLE_BINDINGS_PYTHON)
82-
message(STATUS "Failed to enable Python API due to the 'MLIR_ENABLE_BINDINGS_PYTHON' for LLVM is not ON.")
83-
set(GC_ENABLE_BINDINGS_PYTHON OFF CACHE BOOL "" FORCE)
84-
endif()
85-
86-
if(GC_ENABLE_BINDINGS_PYTHON)
87-
include(MLIRDetectPythonEnv)
88-
mlir_configure_python_dev_packages()
8956
endif()
57+
################################################################################
9058

91-
include_directories(
92-
${LLVM_INCLUDE_DIRS}
93-
${MLIR_INCLUDE_DIRS}
94-
${PROJECT_BINARY_DIR}/include
95-
${PROJECT_SOURCE_DIR}/include
59+
############################## Targets #########################################
60+
# All common options, includes etc. are added to this interface target.
61+
add_library(GcInterface INTERFACE)
62+
target_compile_features(GcInterface INTERFACE cxx_std_17)
63+
target_include_directories(GcInterface INTERFACE
64+
$<BUILD_INTERFACE:${PROJECT_BINARY_DIR}/include>
65+
$<BUILD_INTERFACE:${PROJECT_SOURCE_DIR}/include>
66+
$<INSTALL_INTERFACE:include>
9667
)
9768

98-
add_definitions(${LLVM_DEFINITIONS})
99-
100-
set(GC_MLIR_CXX_FLAGS "")
101-
# Silence a false positive GCC -Wunused-but-set-parameter warning in constexpr
102-
# cases, by marking SelectedCase as used. See
103-
# https://gcc.gnu.org/bugzilla/show_bug.cgi?id=85827 for details. The issue is
104-
# fixed in GCC 10.
105-
if(CMAKE_CXX_COMPILER_ID MATCHES "GNU" AND CMAKE_CXX_COMPILER_VERSION VERSION_LESS "10.0")
106-
check_cxx_compiler_flag("-Wno-unused-but-set-parameter" CXX_SUPPORTS_WNO_UNUSED_BUT_SET_PARAMETER)
107-
append_if(CXX_SUPPORTS_WNO_UNUSED_BUT_SET_PARAMETER "-Wno-unused-but-set-parameter" GC_MLIR_CXX_FLAGS)
108-
endif()
109-
include("cmake/version.cmake")
69+
include(functions)
70+
include(version)
71+
include(mlir)
11072

11173
add_subdirectory(include)
11274
add_subdirectory(lib)
11375
add_subdirectory(src)
114-
if(GC_ENABLE_BINDINGS_PYTHON)
115-
message(STATUS "Enabling Python API")
116-
add_subdirectory(python)
117-
endif()
76+
add_subdirectory(python)
11877
add_subdirectory(test)
78+
################################################################################
11979

120-
# Export the targets
121-
add_library(GcInterface INTERFACE)
122-
target_include_directories(GcInterface INTERFACE
123-
$<BUILD_INTERFACE:${PROJECT_BINARY_DIR}/include>
124-
$<BUILD_INTERFACE:${PROJECT_SOURCE_DIR}/include>
125-
$<INSTALL_INTERFACE:include>
126-
)
80+
############################### Install ########################################
12781
install(DIRECTORY ${PROJECT_SOURCE_DIR}/include/ TYPE INCLUDE)
12882
install(DIRECTORY ${PROJECT_BINARY_DIR}/include/ TYPE INCLUDE
129-
PATTERN "CMake*" EXCLUDE PATTERN "*.cmake" EXCLUDE)
130-
install(TARGETS
131-
GcCpuRuntime
132-
GcGpuPasses
133-
GcInterface
134-
GcJitWrapper
135-
GcPasses
136-
GcUtilsIR
137-
MLIRCPURuntimeDialect
138-
MLIRCPURuntimeTransforms
139-
MLIRLinalgx
140-
MLIRMicrokernel
141-
${GC_OPT_EXE_NAME}
142-
${GC_ONEDNN_DIALECT_LIB_NAME}
83+
REGEX "CMake.*|.*cmake" EXCLUDE)
14384

144-
EXPORT ${PROJECT_NAME}Targets
145-
ARCHIVE DESTINATION ${CMAKE_INSTALL_LIBDIR}
146-
LIBRARY DESTINATION ${CMAKE_INSTALL_LIBDIR}
147-
RUNTIME DESTINATION ${CMAKE_INSTALL_BINDIR}
148-
INCLUDES DESTINATION ${CMAKE_INSTALL_INCLUDEDIR}
149-
PUBLIC_HEADER DESTINATION ${CMAKE_INSTALL_INCLUDEDIR}
85+
# Export the targets
86+
get_property(GC_TOOLS GLOBAL PROPERTY GC_TOOLS)
87+
get_property(GC_MLIR_LIBS GLOBAL PROPERTY GC_MLIR_LIBS)
88+
get_property(GC_PASS_LIBS GLOBAL PROPERTY GC_PASS_LIBS)
89+
get_property(GC_DIALECT_LIBS GLOBAL PROPERTY GC_DIALECT_LIBS)
90+
install(TARGETS
91+
GcInterface
92+
${GC_TOOLS}
93+
${GC_MLIR_LIBS}
94+
${GC_PASS_LIBS}
95+
${GC_DIALECT_LIBS}
96+
EXPORT ${PROJECT_NAME}Targets
97+
ARCHIVE DESTINATION ${CMAKE_INSTALL_LIBDIR}
98+
LIBRARY DESTINATION ${CMAKE_INSTALL_LIBDIR}
99+
RUNTIME DESTINATION ${CMAKE_INSTALL_BINDIR}
100+
INCLUDES DESTINATION ${CMAKE_INSTALL_INCLUDEDIR}
101+
PUBLIC_HEADER DESTINATION ${CMAKE_INSTALL_INCLUDEDIR}
150102
)
151103
export(EXPORT ${PROJECT_NAME}Targets
152-
FILE "${PROJECT_BINARY_DIR}/${PROJECT_NAME}Targets.cmake"
104+
FILE "${PROJECT_BINARY_DIR}/${PROJECT_NAME}Targets.cmake"
153105
)
154106
install(EXPORT ${PROJECT_NAME}Targets
155-
FILE ${PROJECT_NAME}Targets.cmake
156-
DESTINATION lib/cmake/${PROJECT_NAME}
107+
FILE ${PROJECT_NAME}Targets.cmake
108+
DESTINATION lib/cmake/${PROJECT_NAME}
157109
)
158110

159111
# Generate the config files
160112
include(CMakePackageConfigHelpers)
161113
configure_package_config_file(
162-
${PROJECT_SOURCE_DIR}/cmake/Config.cmake.in
163-
"${PROJECT_BINARY_DIR}/${PROJECT_NAME}Config.cmake"
164-
INSTALL_DESTINATION "lib/cmake/${PROJECT_NAME}"
165-
NO_SET_AND_CHECK_MACRO
166-
NO_CHECK_REQUIRED_COMPONENTS_MACRO
114+
${PROJECT_SOURCE_DIR}/cmake/Config.cmake.in
115+
"${PROJECT_BINARY_DIR}/${PROJECT_NAME}Config.cmake"
116+
INSTALL_DESTINATION "lib/cmake/${PROJECT_NAME}"
117+
NO_SET_AND_CHECK_MACRO
118+
NO_CHECK_REQUIRED_COMPONENTS_MACRO
167119
)
168120
write_basic_package_version_file(
169-
"${PROJECT_BINARY_DIR}/${PROJECT_NAME}ConfigVersion.cmake"
170-
COMPATIBILITY AnyNewerVersion
121+
"${PROJECT_BINARY_DIR}/${PROJECT_NAME}ConfigVersion.cmake"
122+
COMPATIBILITY AnyNewerVersion
171123
)
172124
install(FILES
173-
${PROJECT_BINARY_DIR}/${PROJECT_NAME}Config.cmake
174-
${PROJECT_BINARY_DIR}/${PROJECT_NAME}ConfigVersion.cmake
175-
DESTINATION "lib/cmake/${PROJECT_NAME}"
125+
${PROJECT_BINARY_DIR}/${PROJECT_NAME}Config.cmake
126+
${PROJECT_BINARY_DIR}/${PROJECT_NAME}ConfigVersion.cmake
127+
DESTINATION "lib/cmake/${PROJECT_NAME}"
176128
)
129+
################################################################################

‎cmake/Config.cmake.in

+6-1
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,10 @@
11
@PACKAGE_INIT@
22

3-
include ( "${CMAKE_CURRENT_LIST_DIR}/@[email protected]" )
3+
include ("${CMAKE_CURRENT_LIST_DIR}/@[email protected]")
44

55
find_package(MLIR REQUIRED CONFIG)
6+
7+
include_directories(
8+
${LLVM_INCLUDE_DIRS}
9+
${MLIR_INCLUDE_DIRS}
10+
)

‎cmake/functions.cmake

+28
Original file line numberDiff line numberDiff line change
@@ -90,3 +90,31 @@ macro(gc_set_mlir_link_components VAR)
9090
)
9191
endif()
9292
endmacro()
93+
94+
function(gc_add_mlir_library name)
95+
add_mlir_library(${ARGV})
96+
97+
if(name MATCHES ".+Passes")
98+
set_property(GLOBAL APPEND PROPERTY GC_PASS_LIBS ${name})
99+
else()
100+
set_property(GLOBAL APPEND PROPERTY GC_MLIR_LIBS ${name})
101+
endif()
102+
103+
if(GcInterface IN_LIST ARGN)
104+
if(SHARED IN_LIST ARGN)
105+
target_link_libraries(${name} PUBLIC GcInterface)
106+
else()
107+
target_link_libraries(obj.${name} PUBLIC GcInterface)
108+
endif()
109+
endif()
110+
endfunction()
111+
112+
function(gc_add_mlir_dialect_library name)
113+
add_mlir_dialect_library(${ARGV})
114+
target_link_libraries(obj.${name} PUBLIC GcInterface)
115+
set_property(GLOBAL APPEND PROPERTY GC_DIALECT_LIBS ${name})
116+
117+
if(GcInterface IN_LIST ARGN)
118+
target_link_libraries(obj.${name} PUBLIC GcInterface)
119+
endif()
120+
endfunction()

‎cmake/gtest.cmake

+4-4
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
include_guard()
2-
include(functions)
32

43
gc_fetch_content(
5-
GTest
6-
v1.14.0
7-
https://github.com/google/googletest.git
4+
GTest
5+
v1.14.0
6+
https://github.com/google/googletest.git
7+
SET INSTALL_GTEST=OFF
88
)

‎cmake/mlir.cmake

+31
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
include_guard()
2+
3+
find_package(MLIR REQUIRED CONFIG)
4+
5+
message(STATUS "Using MLIRConfig.cmake in: ${MLIR_DIR}")
6+
message(STATUS "Using LLVMConfig.cmake in: ${LLVM_DIR}")
7+
8+
set(LLVM_RUNTIME_OUTPUT_INTDIR ${PROJECT_BINARY_DIR}/bin)
9+
set(LLVM_LIBRARY_OUTPUT_INTDIR ${PROJECT_BINARY_DIR}/lib)
10+
set(MLIR_BINARY_DIR ${PROJECT_BINARY_DIR})
11+
12+
list(APPEND CMAKE_MODULE_PATH "${MLIR_CMAKE_DIR}")
13+
list(APPEND CMAKE_MODULE_PATH "${LLVM_CMAKE_DIR}")
14+
15+
include(TableGen)
16+
include(AddLLVM)
17+
include(AddMLIR)
18+
include(HandleLLVMOptions)
19+
20+
include_directories(
21+
${LLVM_INCLUDE_DIRS}
22+
${MLIR_INCLUDE_DIRS}
23+
)
24+
25+
set(LLVM_TABLEGEN_FLAGS
26+
-I${PROJECT_BINARY_DIR}/include
27+
-I${PROJECT_SOURCE_DIR}/include
28+
)
29+
30+
string(REPLACE " " ";" GC_LLVM_DEFINITIONS ${LLVM_DEFINITIONS})
31+
target_compile_options(GcInterface INTERFACE ${GC_LLVM_DEFINITIONS})

‎cmake/onednn.cmake

+7-9
Original file line numberDiff line numberDiff line change
@@ -1,21 +1,19 @@
11
include_guard()
22

3-
get_property(DNNL_INCLUDES GLOBAL PROPERTY DNNL_INCLUDES)
4-
if (NOT DEFINED DNNL_INCLUDES)
5-
include(functions)
6-
3+
get_property(GC_DNNL_INCLUDES GLOBAL PROPERTY GC_DNNL_INCLUDES)
4+
if (NOT DEFINED GC_DNNL_INCLUDES)
75
# TODO: Change to main https://github.com/oneapi-src/oneDNN.git when all the
86
# required functionality is merged.
97
gc_fetch_content(dnnl dev https://github.com/kurapov-peter/oneDNN.git
108
SKIP_ADD
119
)
1210

13-
set(DNNL_INCLUDES
14-
${dnnl_BINARY_DIR}/include
15-
${dnnl_SOURCE_DIR}/include
16-
${dnnl_SOURCE_DIR}/src
11+
set(GC_DNNL_INCLUDES
12+
$<BUILD_INTERFACE:${dnnl_BINARY_DIR}/include>
13+
$<BUILD_INTERFACE:${dnnl_SOURCE_DIR}/include>
14+
$<BUILD_INTERFACE:${dnnl_SOURCE_DIR}/src>
1715
)
18-
set_property(GLOBAL PROPERTY DNNL_INCLUDES ${DNNL_INCLUDES})
16+
set_property(GLOBAL PROPERTY GC_DNNL_INCLUDES ${GC_DNNL_INCLUDES})
1917

2018
# This allows to generate headers from *.in without adding the library to the build.
2119
# If the build is required, remove this and the SKIP_ADD option above.

‎cmake/version.cmake

+5-5
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,8 @@ if(NOT GIT_FOUND OR RESULT)
1818
set(GC_VERSION_HASH "N/A")
1919
endif()
2020

21-
add_compile_definitions(
22-
GC_VERSION_MAJOR=${GC_VERSION_MAJOR}
23-
GC_VERSION_MINOR=${GC_VERSION_MINOR}
24-
GC_VERSION_PATCH=${GC_VERSION_PATCH}
25-
GC_VERSION_HASH="${GC_VERSION_HASH}")
21+
target_compile_options(GcInterface INTERFACE
22+
-DGC_VERSION_MAJOR=${GC_VERSION_MAJOR}
23+
-DGC_VERSION_MINOR=${GC_VERSION_MINOR}
24+
-DGC_VERSION_PATCH=${GC_VERSION_PATCH}
25+
-DGC_VERSION_HASH="${GC_VERSION_HASH}")

‎include/gc/Dialect/OneDNNGraph/CMakeLists.txt

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
if (NOT GC_ENABLE_TEST_DNNL)
1+
if (NOT GC_ENABLE_DNNL)
22
message(STATUS "OneDNNGraphDialect is not enabled.")
33
return()
44
endif ()

‎include/gc/Transforms/CMakeLists.txt

+2-2
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
if(GC_ENABLE_DNNL)
2-
set(TABLEGEN_MACROS "${TABLEGEN_MACROS} -DGC_HAS_ONEDNN_DIALECT")
2+
list(APPEND TABLEGEN_MACROS -DGC_HAS_ONEDNN_DIALECT)
33
endif()
44
if(GC_ENABLE_GPU)
5-
set(TABLEGEN_MACROS "${TABLEGEN_MACROS} -DGC_USE_GPU")
5+
list(APPEND TABLEGEN_MACROS -DGC_USE_GPU)
66
endif()
77

88
set(LLVM_TARGET_DEFINITIONS Passes.td)

‎lib/gc/CAPI/CMakeLists.txt

+4-3
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,10 @@
11
set(GC_ALL_LIBS
2-
MLIROneDNNGraph
32
${GC_ONEDNN_DIALECT_LIB_NAME}
43
GcPasses
54
MLIRCPURuntimeTransforms)
65

7-
if(GC_USE_GPU)
8-
list(APPEND GC_ALL_LIBS GcGPUPasses)
6+
if(GC_ENABLE_GPU)
7+
list(APPEND GC_ALL_LIBS GcGpuPasses)
98
endif()
109

1110
add_mlir_public_c_api_library(GcCAPI
@@ -14,3 +13,5 @@ add_mlir_public_c_api_library(GcCAPI
1413
LINK_LIBS PUBLIC
1514
${GC_ALL_LIBS}
1615
)
16+
target_link_libraries(obj.GcCAPI PUBLIC GcInterface)
17+
set_property(GLOBAL APPEND PROPERTY GC_MLIR_LIBS GcCAPI)

0 commit comments

Comments
 (0)