From f76391c04c127d8bfb7423f05c7e3d759c51dda3 Mon Sep 17 00:00:00 2001
From: lhy <442488254@qq.com>
Date: Tue, 7 Nov 2023 14:01:32 +0800
Subject: [PATCH 1/4] make the diopi_impl as an INTERFACE so users can decide
 whether to link it.

---
 dipu/third_party/CMakeLists.txt | 28 +++++++++++++++++++++-------
 1 file changed, 21 insertions(+), 7 deletions(-)

diff --git a/dipu/third_party/CMakeLists.txt b/dipu/third_party/CMakeLists.txt
index 78e788ecf..beb87c81f 100644
--- a/dipu/third_party/CMakeLists.txt
+++ b/dipu/third_party/CMakeLists.txt
@@ -1,8 +1,7 @@
 #require the external project to build as target
 include(ExternalProject)
 
-add_library(diopi_impl SHARED IMPORTED GLOBAL)
-
+add_library(diopi_impl_lib SHARED IMPORTED GLOBAL)
 if((${WITH_DIOPI} STREQUAL "INTERNAL") OR (${WITH_DIOPI} STREQUAL "internal"))
   if(NOT ${DIOPI_IMPL_OPT} STREQUAL "")
 #----------------------------------Build DIOPI submodule-------------------------------------------
@@ -36,10 +35,11 @@ if((${WITH_DIOPI} STREQUAL "INTERNAL") OR (${WITH_DIOPI} STREQUAL "internal"))
     ExternalProject_Add_StepTargets(diopi_internal configure build install)
     ExternalProject_Add_StepDependencies(diopi_internal install diopi_internal-build)
     ExternalProject_Add_StepDependencies(diopi_internal build diopi_internal-configure)
-    add_dependencies(diopi_impl diopi_internal-install)
+    add_dependencies(diopi_impl_lib diopi_internal-install)
   endif()
 #--------------------------------------------------------------------------------------------------
-else()
+elseif(NOT ${WITH_DIOPI} STREQUAL "OFF")
+  #WITH_DIOPI=OFF means does not need DIOPI
   if(EXISTS $ENV{DIOPI_PATH})
     set(DIOPI_PROTO_PATH $ENV{DIOPI_PATH})
   else()
@@ -52,6 +52,7 @@ else()
     set(DIOPI_IMPL_LIB_PATH "${WITH_DIOPI}/impl/lib")
     set(DIOPI_IMPL_LIB "${DIOPI_IMPL_LIB_PATH}/libdiopi_impl.so")
   endif()
+  #WITH_DIOPI=OFF means does not need DIOPI
   find_library(FIND_DIOPI_IMPL_LIB_RESULTS 
                NAMES libdiopi_impl 
                      libdiopi_impl.so 
@@ -59,16 +60,29 @@ else()
                      diopi_impl.so
                HINTS ${DIOPI_IMPL_LIB_PATH}
                REQUIRED)
+else()
+  if(EXISTS $ENV{DIOPI_PATH})
+    set(DIOPI_PROTO_PATH $ENV{DIOPI_PATH})
+  else()
+    set(DIOPI_PROTO_PATH "${PROJECT_SOURCE_DIR}/third_party/DIOPI/proto")
+  endif()
 endif()
 
 message(STATUS "DIOPI_PROTO_PATH: ${DIOPI_PROTO_PATH}")
 message(STATUS "DIOPI_IMPL_LIB_PATH: ${DIOPI_IMPL_LIB_PATH}")
 
-set_target_properties(
-  diopi_impl PROPERTIES IMPORTED_LOCATION
-                        ${DIOPI_IMPL_LIB})
+add_library(diopi_impl INTERFACE)
+target_link_options(diopi_impl INTERFACE "LINKER:-no-as-needed")
 target_include_directories(diopi_impl SYSTEM INTERFACE ${DIOPI_PROTO_PATH}/include)
 target_compile_definitions(diopi_impl INTERFACE DIOPI_ATTR_WEAK)
+
+#WITH_DIOPI=OFF means does not need DIOPI
+if(NOT ${WITH_DIOPI} STREQUAL "OFF")
+  set_target_properties(
+    diopi_impl_lib PROPERTIES IMPORTED_LOCATION
+                          ${DIOPI_IMPL_LIB})
+  target_link_libraries(diopi_impl INTERFACE diopi_impl_lib)
+endif()
 #-----------------------------------------------------------------------------------------------
 
 #-------------------------add kineto as an external project ------------------------------------

From e987fd86c72e93ef9dccc965a786dd6faf30e88a Mon Sep 17 00:00:00 2001
From: lhy <442488254@qq.com>
Date: Tue, 7 Nov 2023 14:36:50 +0800
Subject: [PATCH 2/4] improve the code.

---
 dipu/third_party/CMakeLists.txt | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/dipu/third_party/CMakeLists.txt b/dipu/third_party/CMakeLists.txt
index beb87c81f..1201b6d00 100644
--- a/dipu/third_party/CMakeLists.txt
+++ b/dipu/third_party/CMakeLists.txt
@@ -38,8 +38,8 @@ if((${WITH_DIOPI} STREQUAL "INTERNAL") OR (${WITH_DIOPI} STREQUAL "internal"))
     add_dependencies(diopi_impl_lib diopi_internal-install)
   endif()
 #--------------------------------------------------------------------------------------------------
-elseif(NOT ${WITH_DIOPI} STREQUAL "OFF")
-  #WITH_DIOPI=OFF means does not need DIOPI
+elseif(NOT ((${WITH_DIOPI} STREQUAL "OFF") OR (${WITH_DIOPI} STREQUAL "off")) )
+  #WITH_DIOPI=OFF means does not link DIOPI
   if(EXISTS $ENV{DIOPI_PATH})
     set(DIOPI_PROTO_PATH $ENV{DIOPI_PATH})
   else()
@@ -52,7 +52,6 @@ elseif(NOT ${WITH_DIOPI} STREQUAL "OFF")
     set(DIOPI_IMPL_LIB_PATH "${WITH_DIOPI}/impl/lib")
     set(DIOPI_IMPL_LIB "${DIOPI_IMPL_LIB_PATH}/libdiopi_impl.so")
   endif()
-  #WITH_DIOPI=OFF means does not need DIOPI
   find_library(FIND_DIOPI_IMPL_LIB_RESULTS 
                NAMES libdiopi_impl 
                      libdiopi_impl.so 
@@ -72,12 +71,12 @@ message(STATUS "DIOPI_PROTO_PATH: ${DIOPI_PROTO_PATH}")
 message(STATUS "DIOPI_IMPL_LIB_PATH: ${DIOPI_IMPL_LIB_PATH}")
 
 add_library(diopi_impl INTERFACE)
-target_link_options(diopi_impl INTERFACE "LINKER:-no-as-needed")
 target_include_directories(diopi_impl SYSTEM INTERFACE ${DIOPI_PROTO_PATH}/include)
 target_compile_definitions(diopi_impl INTERFACE DIOPI_ATTR_WEAK)
 
-#WITH_DIOPI=OFF means does not need DIOPI
-if(NOT ${WITH_DIOPI} STREQUAL "OFF")
+#WITH_DIOPI=OFF means does not link DIOPI
+if(NOT ((${WITH_DIOPI} STREQUAL "OFF") OR (${WITH_DIOPI} STREQUAL "off")) )
+  target_link_options(diopi_impl_lib INTERFACE "LINKER:-no-as-needed")
   set_target_properties(
     diopi_impl_lib PROPERTIES IMPORTED_LOCATION
                           ${DIOPI_IMPL_LIB})

From d3a4bbbd7cbd7d2ccf3cf16849a7e868caac061e Mon Sep 17 00:00:00 2001
From: wiryls <7984500+wiryls@users.noreply.github.com>
Date: Mon, 13 Nov 2023 14:12:02 +0800
Subject: [PATCH 3/4] feat: splite WITH_DIOPI into two options

---
 dipu/scripts/ci/droplet/ci_droplet_script.sh |  19 +--
 dipu/third_party/CMakeLists.txt              | 137 ++++++++++---------
 2 files changed, 86 insertions(+), 70 deletions(-)

diff --git a/dipu/scripts/ci/droplet/ci_droplet_script.sh b/dipu/scripts/ci/droplet/ci_droplet_script.sh
index 0a1cf53ed..7f16e5080 100644
--- a/dipu/scripts/ci/droplet/ci_droplet_script.sh
+++ b/dipu/scripts/ci/droplet/ci_droplet_script.sh
@@ -17,16 +17,19 @@ function config_dipu_droplet_cmake() {
 }
 
 function config_all_droplet_cmake() {
-    mkdir -p build && cd ./build && rm -rf ./*
+    rm -rf ./build
+    mkdir -p build
     echo "config_dipu_nv_cmake PYTORCH_DIR: ${PYTORCH_DIR}"
     echo "config_dipu_nv_cmake PYTHON_INCLUDE_DIR: ${PYTHON_INCLUDE_DIR}"
-    cmake ../  -DCMAKE_BUILD_TYPE=Debug \
-     -DDEVICE=${DIPU_DEVICE} -DPYTORCH_DIR=${PYTORCH_DIR} \
-     -DWITH_DIOPI="none" \
-     -DPYTHON_INCLUDE_DIR=${PYTHON_INCLUDE_DIR}
-      # -DCMAKE_C_FLAGS_DEBUG="-g -O0" \
-      # -DCMAKE_CXX_FLAGS_DEBUG="-g -O0"
-    cd ../
+
+    args=(
+        "-DCMAKE_BUILD_TYPE=Debug"
+        "-DDEVICE=${DIPU_DEVICE}"
+        "-DPYTORCH_DIR=${PYTORCH_DIR}"
+        "-DPYTHON_INCLUDE_DIR=${PYTHON_INCLUDE_DIR}" )
+    [[ ! -z "$DIOPI_ROOT" ]] && args+=( "-DWITH_DIOPI_LIBRARY=${DIOPI_ROOT}" )
+    [[ ! -z "$DIOPI_PATH" ]] && args+=( "-DWITH_DIOPI_INCLUDE=${DIOPI_PATH}/include" )
+    cmake -S . -B build "${args[@]}"
 }
 
 function build_diopi_lib() {
diff --git a/dipu/third_party/CMakeLists.txt b/dipu/third_party/CMakeLists.txt
index ab835752d..f20d1d762 100644
--- a/dipu/third_party/CMakeLists.txt
+++ b/dipu/third_party/CMakeLists.txt
@@ -1,75 +1,88 @@
 #require the external project to build as target
 include(ExternalProject)
 
-add_library(diopi_impl SHARED IMPORTED GLOBAL)
+#[[ DIOPI ]]
 
-if(("${WITH_DIOPI}" STREQUAL "INTERNAL") OR ("${WITH_DIOPI}" STREQUAL "internal"))
-  if(NOT ${DIOPI_IMPL_OPT} STREQUAL "")
-#----------------------------------Build DIOPI submodule-------------------------------------------
-    if(NOT DEFINED DIOPI_CMAKE_PREFIX_PATH)
-      execute_process(
-        COMMAND
-          sh -x -c
-          "python -c 'import torch;print(torch.utils.cmake_prefix_path)'"
-        OUTPUT_VARIABLE DIOPI_CMAKE_PREFIX_PATH
-        OUTPUT_STRIP_TRAILING_WHITESPACE)
-    endif()
-  
-    message(STATUS "building internal DIOPI")
-    set(DIOPI_SRC_PATH "${PROJECT_SOURCE_DIR}/third_party/DIOPI")
-    set(DIOPI_BUILD_PATH "${DIOPI_SRC_PATH}/build")
-    set(DIOPI_PROTO_PATH "${DIOPI_SRC_PATH}/proto")
-    set(DIOPI_IMPL_LIB_PATH "${DIOPI_SRC_PATH}/impl/lib")
-    set(DIOPI_IMPL_LIB "${DIOPI_IMPL_LIB_PATH}/libdiopi_impl.so")
-    ExternalProject_Add(diopi_internal
-                        SOURCE_DIR ${DIOPI_SRC_PATH}
-                        SOURCE_SUBDIR impl
-                        BINARY_DIR ${DIOPI_BUILD_PATH}
-                        DOWNLOAD_COMMAND ""
-                        CMAKE_ARGS -DIMPL_OPT=${DIOPI_IMPL_OPT}
-                                   -DENABLE_COVERAGE=${USE_COVERAGE} 
-                                   -DCMAKE_PREFIX_PATH=${DIOPI_CMAKE_PREFIX_PATH}
-                        BUILD_BYPRODUCTS ${DIOPI_IMPL_LIB}
-                        INSTALL_COMMAND cmake -E echo "Skipping install step for diopi_internal."
-    )
-    ## The following code is a work around to avoid make file to run multiple externalProject-build when using make -j N
-    ExternalProject_Add_StepTargets(diopi_internal configure build install)
-    ExternalProject_Add_StepDependencies(diopi_internal install diopi_internal-build)
-    ExternalProject_Add_StepDependencies(diopi_internal build diopi_internal-configure)
-    add_dependencies(diopi_impl diopi_internal-install)
-  endif()
-#--------------------------------------------------------------------------------------------------
+# define some shared and cached options
+set(WITH_DIOPI_LIBRARY "INTERNAL" CACHE STRING
+  "Decide how to link DIOPI library, it could be one of (case-sensitive) 'INTERNAL', 'DISABLE' or '/directory/of/an/external/DIOPI/library'. \
+  It's INTERNAL by default and uses the internal DIOPI library. \
+  When it's DISABLE, DIOPI won't link to DIPU. \
+  When it's an absolute path, it use a user-provided DIOPI library.")
+set(WITH_DIOPI_INCLUDE "" CACHE PATH # use "PATH" type to provide a GUI file selector and convert relative path into absolute path
+  "Provide a folder of external DIOPI header files (or use internal DIOPI if is empty)")
+
+# locate DIOPI_LIBRARY_PATH
+if(WITH_DIOPI_LIBRARY STREQUAL "INTERNAL")
+  set(DIOPI_LIBRARY_PATH "${CMAKE_CURRENT_SOURCE_DIR}/DIOPI/impl/lib")
+  # the default path is hard-coded and not safe, better to use other methods
+elseif(WITH_DIOPI_LIBRARY STREQUAL "DISABLE")
+  set(DIOPI_LIBRARY_PATH "")
+elseif(EXISTS "${WITH_DIOPI_LIBRARY}" AND IS_DIRECTORY "${WITH_DIOPI_LIBRARY}")
+  set(DIOPI_LIBRARY_PATH "${WITH_DIOPI_LIBRARY}")
 else()
-  if(EXISTS $ENV{DIOPI_PATH})
-    set(DIOPI_PROTO_PATH $ENV{DIOPI_PATH})
-  else()
-    set(DIOPI_PROTO_PATH "${WITH_DIOPI}/proto")
-  endif()
-  if(EXISTS $ENV{DIOPI_ROOT})
-    set(DIOPI_IMPL_LIB_PATH $ENV{DIOPI_ROOT})
-    set(DIOPI_IMPL_LIB "${DIOPI_IMPL_LIB_PATH}/libdiopi_impl.so")
-  else()
-    set(DIOPI_IMPL_LIB_PATH "${WITH_DIOPI}/impl/lib")
-    set(DIOPI_IMPL_LIB "${DIOPI_IMPL_LIB_PATH}/libdiopi_impl.so")
+  message(FATAL_ERROR
+    "WITH_DIOPI_LIBRARY is invalid ('${WITH_DIOPI_LIBRARY}'), "
+    "it should be one of 'INTERNAL', 'DISABLE' or an absolute path")
+endif()
+
+# locate DIOPI_INCLUDE_PATH
+if (WITH_DIOPI_INCLUDE STREQUAL "")
+  set(DIOPI_INCLUDE_PATH "${CMAKE_CURRENT_SOURCE_DIR}/DIOPI/proto/include")
+  # the default path is hard-coded and not safe, better to use other methods
+elseif(EXISTS "${WITH_DIOPI_INCLUDE}" AND IS_DIRECTORY "${WITH_DIOPI_INCLUDE}")
+  set(DIOPI_INCLUDE_PATH "${WITH_DIOPI_INCLUDE}")
+else()
+  message(FATAL_ERROR
+    "WITH_DIOPI_INCLUDE is invalid ('${WITH_DIOPI_INCLUDE}'), "
+    "it should be empty or '/directory/of/DIOPI/header/files'")
+endif()
+
+# compile DIOPI if use internal one
+if (WITH_DIOPI_LIBRARY STREQUAL "INTERNAL")
+  if(NOT DEFINED DIOPI_CMAKE_PREFIX_PATH)
+    execute_process(
+      COMMAND
+        sh -x -c
+        "python -c 'import torch;print(torch.utils.cmake_prefix_path)'"
+      OUTPUT_VARIABLE DIOPI_CMAKE_PREFIX_PATH
+      OUTPUT_STRIP_TRAILING_WHITESPACE)
   endif()
-  find_library(FIND_DIOPI_IMPL_LIB_RESULTS 
-               NAMES libdiopi_impl 
-                     libdiopi_impl.so 
-                     diopi_impl 
-                     diopi_impl.so
-               HINTS ${DIOPI_IMPL_LIB_PATH}
-               REQUIRED)
+
+  message(STATUS "Building internal DIOPI with DIOPI_IMPL_OPT: ${DIOPI_IMPL_OPT}")
+  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 args (such as "${DIOPI_IMPL_OPT}").
+                        -DIMPL_OPT=${DIOPI_IMPL_OPT}
+                        -DENABLE_COVERAGE=${USE_COVERAGE}
+                        -DCMAKE_PREFIX_PATH=${DIOPI_CMAKE_PREFIX_PATH}
+                      BUILD_BYPRODUCTS "${DIOPI_LIBRARY_PATH}/libdiopi_impl.so"
+                      INSTALL_COMMAND cmake -E echo "Skipping install step for diopi_internal.")
+
+  ## The following code is a work around to avoid make file to run multiple externalProject-build when using make -j N
+  ExternalProject_Add_StepTargets(diopi_internal configure build install)
+  ExternalProject_Add_StepDependencies(diopi_internal install diopi_internal-build)
+  ExternalProject_Add_StepDependencies(diopi_internal build diopi_internal-configure)
 endif()
 
-message(STATUS "DIOPI_PROTO_PATH: ${DIOPI_PROTO_PATH}")
-message(STATUS "DIOPI_IMPL_LIB_PATH: ${DIOPI_IMPL_LIB_PATH}")
+message(STATUS "Using DIOPI_LIBRARY_PATH='${DIOPI_LIBRARY_PATH}', DIOPI_INCLUDE_PATH='${DIOPI_INCLUDE_PATH}'")
 
-set_target_properties(
-  diopi_impl PROPERTIES IMPORTED_LOCATION
-                        ${DIOPI_IMPL_LIB})
-target_include_directories(diopi_impl SYSTEM INTERFACE ${DIOPI_PROTO_PATH}/include)
+add_library(diopi_impl INTERFACE)
+target_include_directories(diopi_impl SYSTEM INTERFACE ${DIOPI_INCLUDE_PATH})
 target_compile_definitions(diopi_impl INTERFACE DIOPI_ATTR_WEAK)
-#-----------------------------------------------------------------------------------------------
+
+if(NOT WITH_DIOPI_LIBRARY STREQUAL "DISABLE")
+  add_library(diopi_impl_lib SHARED IMPORTED GLOBAL)
+  target_link_options(diopi_impl_lib INTERFACE "LINKER:-no-as-needed")
+  set_target_properties(diopi_impl_lib PROPERTIES IMPORTED_LOCATION "${DIOPI_LIBRARY_PATH}/libdiopi_impl.so")
+
+  add_dependencies(diopi_impl_lib diopi_internal-install)
+  target_link_libraries(diopi_impl INTERFACE diopi_impl_lib)
+endif()
 
 #-------------------------add kineto as an external project ------------------------------------
 #-------------------------use the local submodule(without download)-----------------------------

From 8e9a7839138dd9f83b07cea1e5d9f861ef127e45 Mon Sep 17 00:00:00 2001
From: wiryls <7984500+wiryls@users.noreply.github.com>
Date: Mon, 13 Nov 2023 18:43:42 +0800
Subject: [PATCH 4/4] fix: add one more check and remove GLOBAL

---
 dipu/third_party/CMakeLists.txt | 18 ++++++++++--------
 1 file changed, 10 insertions(+), 8 deletions(-)

diff --git a/dipu/third_party/CMakeLists.txt b/dipu/third_party/CMakeLists.txt
index f20d1d762..dbb835b0b 100644
--- a/dipu/third_party/CMakeLists.txt
+++ b/dipu/third_party/CMakeLists.txt
@@ -5,12 +5,14 @@ include(ExternalProject)
 
 # define some shared and cached options
 set(WITH_DIOPI_LIBRARY "INTERNAL" CACHE STRING
-  "Decide how to link DIOPI library, it could be one of (case-sensitive) 'INTERNAL', 'DISABLE' or '/directory/of/an/external/DIOPI/library'. \
+  "Decide how to use DIOPI library, it could be one of (case-sensitive) \
+  'INTERNAL', 'DISABLE' or '/directory/of/an/external/DIOPI/library'. \
   It's INTERNAL by default and uses the internal DIOPI library. \
-  When it's DISABLE, DIOPI won't link to DIPU. \
-  When it's an absolute path, it use a user-provided DIOPI library.")
+  When it's DISABLE, DIPU won't link DIOPI. \
+  When it's an absolute path, a user-provided DIOPI library will be used.")
 set(WITH_DIOPI_INCLUDE "" CACHE PATH # use "PATH" type to provide a GUI file selector and convert relative path into absolute path
-  "Provide a folder of external DIOPI header files (or use internal DIOPI if is empty)")
+  "Provide a directory of external DIOPI header files, or use internal DIOPI if is empty. \
+  When it's a directory, make sure WITH_DIOPI_LIBRARY is not INTERNAL.")
 
 # locate DIOPI_LIBRARY_PATH
 if(WITH_DIOPI_LIBRARY STREQUAL "INTERNAL")
@@ -30,12 +32,12 @@ endif()
 if (WITH_DIOPI_INCLUDE STREQUAL "")
   set(DIOPI_INCLUDE_PATH "${CMAKE_CURRENT_SOURCE_DIR}/DIOPI/proto/include")
   # the default path is hard-coded and not safe, better to use other methods
-elseif(EXISTS "${WITH_DIOPI_INCLUDE}" AND IS_DIRECTORY "${WITH_DIOPI_INCLUDE}")
+elseif(EXISTS "${WITH_DIOPI_INCLUDE}" AND IS_DIRECTORY "${WITH_DIOPI_INCLUDE}" AND NOT WITH_DIOPI_LIBRARY STREQUAL "INTERNAL")
   set(DIOPI_INCLUDE_PATH "${WITH_DIOPI_INCLUDE}")
 else()
   message(FATAL_ERROR
-    "WITH_DIOPI_INCLUDE is invalid ('${WITH_DIOPI_INCLUDE}'), "
-    "it should be empty or '/directory/of/DIOPI/header/files'")
+    "WITH_DIOPI_INCLUDE is invalid ('${WITH_DIOPI_INCLUDE}'). "
+    "It should be empty or '/directory/of/DIOPI/headers' if WITH_DIOPI_LIBRARY is not INTERNAL.")
 endif()
 
 # compile DIOPI if use internal one
@@ -76,7 +78,7 @@ target_include_directories(diopi_impl SYSTEM INTERFACE ${DIOPI_INCLUDE_PATH})
 target_compile_definitions(diopi_impl INTERFACE DIOPI_ATTR_WEAK)
 
 if(NOT WITH_DIOPI_LIBRARY STREQUAL "DISABLE")
-  add_library(diopi_impl_lib SHARED IMPORTED GLOBAL)
+  add_library(diopi_impl_lib SHARED IMPORTED)
   target_link_options(diopi_impl_lib INTERFACE "LINKER:-no-as-needed")
   set_target_properties(diopi_impl_lib PROPERTIES IMPORTED_LOCATION "${DIOPI_LIBRARY_PATH}/libdiopi_impl.so")