关于 CMakeLists.txt 改进问题

[wiryls]

背景

目前各个 CMakeLists.txt 仅仅是可以编译，但埋下的坑比较多。

CMake 本身存在一些奇怪设计以及一些历史问题，这里补充一些背景知识，比如说 CMake 的大量 command (主要是 CMake 2.x 之前的指令) 的影响范围是全局的：

1. 通过 set 构造的变量 (除非它在 function 内) 将会影响当前 CMakeLists.txt 及其全部 subdirectory；
2. 旧版 command 的影响大都是全局的，例如 add_definitions 会影响该行之后的全部 target，因此 modern CMake 建议废除这些 commands，并且使用现在范围的 target_* 版本。

当前问题

• 在 dipu/CMakeLists.txt 引入的 CMake 变量、compile-flags 很容易引起污染，这些变量、flag 等会影响全部的 subdriectory 中的 targets，后续很可能会影响 third_party 中的内容的编译：

  deeplink.framework/dipu/CMakeLists.txt

  Lines 27 to 33 in 55df57b

  	set(DIOPI_IMPL_OPT "")
  	if (${DEVICE} IN_LIST DEVICE_CUDA)
  	set(USE_CUDA ON)
  	set(UsedVendor cuda)
  	set(DIOPI_IMPL_OPT "torch")
  	elseif (${DEVICE} IN_LIST DEVICE_CAMB)
  	set(USE_CAMB ON)

  deeplink.framework/dipu/CMakeLists.txt

  Line 98 in 55df57b

  add_definitions(-fabi-version=${DIPU_ABI_V})

  deeplink.framework/dipu/CMakeLists.txt

  Line 104 in 55df57b

  add_compile_options(-D_GLIBCXX_USE_CXX11_ABI=${DIPU_COMPILED_WITH_CXX11_ABI})

  deeplink.framework/dipu/CMakeLists.txt

  Line 123 in 55df57b

  _set_cpp_flags()

  deeplink.framework/dipu/CMakeLists.txt

  Lines 126 to 127 in 55df57b

  	set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fprofile-arcs -ftest-coverage")
  	set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -fprofile-arcs -ftest-coverage")

  为了避免污染，当前在 third_party 引入第三方库时，直接使用了 ExternalProject_Add：

  deeplink.framework/dipu/third_party/CMakeLists.txt

  Lines 55 to 64 in 55df57b

  	ExternalProject_Add(diopi_internal
  	SOURCE_DIR "${CMAKE_CURRENT_SOURCE_DIR}/DIOPI"
  	SOURCE_SUBDIR "impl"
  	BINARY_DIR "${CMAKE_CURRENT_SOURCE_DIR}/DIOPI/build"
  	DOWNLOAD_COMMAND ""
  	CMAKE_ARGS
  	# note: as CMAKE_ARGS is a list, do not add quotes to arg values (such as "${DIOPI_IMPL_OPT}").
  	"-DIMPL_OPT=${DIOPI_IMPL_OPT}"
  	"-DENABLE_COVERAGE=${USE_COVERAGE}"
  	"-DCMAKE_BUILD_TYPE=${CMAKE_BUILD_TYPE}"

  这种写法能够避免上游传递的各种全局参数的污染，但同时也导致脚本维护变得更加困难，目标项目相当于单独编译了一份，该过程需要手动传递各种参数，手动控制目标的依赖项目及编译顺序。
  • 有机会尽早重构一下，换用 modern 的写法，避免污染，否则越来越难改
  • 目前已经使用 add_subdirectory 引入 kineto，后续继续新增全局的 flag 容易导致问题
  • 手动控制编译过程比较麻烦，例如之前就时不时发生 libfmt 链接失败的问题
• 应该在 CMakeLists.txt 的 option 中加入前缀 (例如 DIPU_BUILD_TESTS)

  deeplink.framework/dipu/CMakeLists.txt

  Lines 5 to 6 in 55df57b

  	option(TESTS "Whether to build unit tests" OFF)
  	option(LIBS "Whether to build dipu lib, default on" ON)

  • 毕竟 option 默认作用域也是全局，此类命名容易与上下游项目发生冲突
  • 需要注意，此类 option 的修改属于 breaking changes
• 应该使用 modern CMake 风格重构该 CMakeLists.txt 文件，移除这类旧版 command：

  deeplink.framework/dipu/torch_dipu/csrc_dipu/CMakeLists.txt

  Line 13 in 55df57b

  include_directories(${DIST_DIR})

  deeplink.framework/dipu/torch_dipu/csrc_dipu/CMakeLists.txt

  Lines 69 to 70 in 55df57b

  	include_directories(SYSTEM ${VENDOR_INCLUDE_DIRS})
  	link_directories(${VENDOR_LIB_DIRS})

• file(GLOB *) 并不可靠，CMake 的官方文档有提到最好不要这样做

  deeplink.framework/dipu/torch_dipu/csrc_dipu/CMakeLists.txt

  Lines 44 to 47 in 6d80677

  	file(
  	GLOB
  	RT_SRC_FILES
  	runtime/core/guardimpl/*.cpp

  • 如果一定要这样做，考虑用 GLOB_RECURSE 简化写法
• 考虑在 CMakeLists.txt 加入平台相关判断及警告，如果只支持 *unix 平台，应该向非该类平台的用户提醒错误信息。目前的各种 command 都默认使用 GCC