Fix cmake for AMDGPU platform #13801

sabreshao · 2018-10-10T06:27:51Z

No description provided.

Enable whole archieve build for pybind library. Disable two warning. Rollback to C++11. [Don't merge!] Link RCCL to WA gpu kernel loading issue. Update eigen to fix build failure. Add more include directories. Fix O3 build failure. Update eigen. fix tensor_util_test segment fault issue add more macro check in hip.cmake. we may consider refine hip.cmake to inherit all add_definitions() in parrent scope, in the future. Signed-off-by: carlushuang <carlus.huang@amd.com> Fix rocRAND load. Update eigen to fix gru_unit_op and reduce_op. Add HIP support to testing. Update eigen to support int16 and int8 in arg min and arg max. Conflicts: paddle/fluid/pybind/CMakeLists.txt Conflicts: paddle/fluid/pybind/CMakeLists.txt

Signed-off-by: carlushuang <carlus.huang@amd.com> Conflicts: paddle/fluid/operators/CMakeLists.txt

sabreshao · 2018-10-15T03:27:49Z

@dzhwinter @luotao1

Conflicts: paddle/fluid/operators/CMakeLists.txt

Conflicts: CMakeLists.txt

Xreki

Sorry for so many questions, because we don't have an AMD GPU, we cannot test the PR.

Xreki · 2018-11-02T03:28:39Z

cmake/external/eigen.cmake

@@ -17,7 +17,7 @@ if(WITH_AMD_GPU)
        extern_eigen3
        ${EXTERNAL_PROJECT_LOG_ARGS}
        GIT_REPOSITORY  "https://github.com/sabreshao/hipeigen.git"
-        GIT_TAG         0cba03ff9f8f9f70bbd92ac5857b031aa8fed6f9


Why change the GIT_TAG?

Two reasones:

Rebase to the eigen used by CUDA build.

Several commits to the repo to fix unittest failure such as test_gru_unit_op and test_reduce_op.

Xreki · 2018-11-02T03:31:05Z

cmake/external/rocprim.cmake

+    GIT_TAG        5bd41b96ab8d8343330fb2c3e1b96775bde3b3fc 
+    PREFIX         ${ROCPRIM_SOURCE_DIR}
+    UPDATE_COMMAND  ""
+    CMAKE_ARGS     -DCMAKE_CXX_COMPILER=/opt/rocm/bin/hcc


Since rocprim is a header-only library, does the set of cxx compiler here have an actual effect? Do the codes which include rocprim also need to be compiled by hcc?

I think it's better not use an absolute path here.

Some header files are generated in building, though compilation is not needed.

Xreki · 2018-11-02T03:34:18Z

cmake/external/rocprim.cmake

@@ -0,0 +1,39 @@
+if (NOT WITH_AMD_GPU)


Would you please add some comment to introduce the package rocprim here?

Xreki · 2018-11-02T03:52:03Z

cmake/external/rocprim.cmake

+
+add_dependencies(rocprim extern_rocprim)
+
+LIST(APPEND external_project_dependencies rocprim)


This line is not needed, because rocprim is not used in Paddle v2.

Xreki · 2018-11-02T03:53:25Z

cmake/generic.cmake

-	target_link_libraries(${TARGET_NAME} ${hip_library_DEPS})
+      if("${hip_library_DEPS}" MATCHES "ARCHIVE_START")
+        # Support linking flags: --whole-archive (Linux) / -force_load (MacOS).
+        # WARNING: Please don't use ARCHIVE_START&ARCHIVE_END if TARGET_NAME will be linked by other libraries.


It is not suggested to use whole-archive here.

That's a WA towards HIP/HCC toolchain, some symbol will be missing without "whole-archive".

Paddle Fluid uses USE_OP to ensure all operators linked, maybe whole-archive here can be removed in the same way. This can be done in the future. By the way, please format the codes in hip_library.

Xreki · 2018-11-02T03:54:38Z

cmake/hip.cmake

+set(HIP_HCC_FLAGS "${HIP_HCC_FLAGS} -fPIC -DPADDLE_WITH_HIP -std=c++11" )
+
+if(NOT WITH_PYTHON)
+  set(HIP_HCC_FLAGS "${HIP_HCC_FLAGS} -DPADDLE_NO_PYTHON")


Where to use HIP_HCC_FLAGS and where to use hcc?

HIP/HCC toolchain will refer the flag HIP_HCC_FLAGS. When a libarry/executable is marked as hip_library/hip_exe, source files with postfix .cu will be compiled by HIP/HCC toolchain.

Xreki · 2018-11-02T03:55:59Z

paddle/fluid/pybind/CMakeLists.txt

-      DEPS ${PYBIND_DEPS}
-      ${GLOB_OP_LIB})
+      DEPS ARCHIVE_START ${PYBIND_DEPS}
+      ${GLOB_OP_LIB} ARCHIVE_END)


Why add ARCHIVE_START here?

That's a WA towards HIP/HCC toolchain, some symbol will be missing without "whole-archive".

Xreki · 2018-11-02T04:43:11Z

paddle/fluid/operators/CMakeLists.txt

@@ -254,6 +254,9 @@ op_library(cross_entropy_op DEPS cross_entropy)
 if(WITH_GPU)
  op_library(softmax_with_cross_entropy_op DEPS cross_entropy softmax cub)
  op_library(sequence_softmax_op DEPS cub)
+elseif(WITH_AMD_GPU)
+  op_library(softmax_with_cross_entropy_op DEPS cross_entropy softmax rocprim)
+  op_library(sequence_softmax_op DEPS rocprim)


The header file is not included in the operators ".cc", so why need to depend on rocprim here?

Instead of including cub, HIP/HCC build will refer to the header files of rocprim.

sabreshao · 2018-11-09T09:01:24Z

@Xreki Can you have a look?

Conflicts: paddle/fluid/operators/CMakeLists.txt paddle/fluid/pybind/CMakeLists.txt paddle/testing/paddle_gtest_main.cc

carlushuang · 2018-11-22T02:55:27Z

@Xreki Hi please take a look at this PR. The Cmake system is at the root of enabling AMDGPU support, and later AMDGPU related code have a strong dependency on this one.

Xreki

Would you please teel me how to update the version of hip? My build failed due to the following error:

CMake Error at /usr/share/cmake-3.5/Modules/FindPackageHandleStandardArgs.cmake:148 (message):                                                                                               
  Could NOT find HIP: Found unsuitable version "1.5.18073", but required is
  at least "1.5.18263" (found /opt/rocm)

Xreki · 2018-11-22T06:50:48Z

cmake/generic.cmake

-	target_link_libraries(${TARGET_NAME} ${hip_library_DEPS})
+      if("${hip_library_DEPS}" MATCHES "ARCHIVE_START")
+        # Support linking flags: --whole-archive (Linux) / -force_load (MacOS).
+        # WARNING: Please don't use ARCHIVE_START&ARCHIVE_END if TARGET_NAME will be linked by other libraries.


Paddle Fluid uses USE_OP to ensure all operators linked, maybe whole-archive here can be removed in the same way. This can be done in the future. By the way, please format the codes in hip_library.

Xreki · 2018-11-22T07:10:34Z

cmake/hip.cmake

+set(HIP_HCC_FLAGS "${HIP_HCC_FLAGS} -fPIC -DPADDLE_WITH_HIP -std=c++11" )
+
+if(NOT WITH_PYTHON)
+  set(HIP_HCC_FLAGS "${HIP_HCC_FLAGS} -DPADDLE_NO_PYTHON")


In fact, PADDLE_NO_PYTHON is only used in legacy Paddle, so there is no need to add this line. So as WITH_TIMER and WITH_DOUBLE.

Useless flag will be removed. Can you point out Paddle coding style, since pre-commit hook doesn't touch cmakefile?

For cmake file, just need to keep the right indentation.

carlushuang · 2018-11-22T08:56:50Z

@Xreki Hi

CMake Error at /usr/share/cmake-3.5/Modules/FindPackageHandleStandardArgs.cmake:148 (message):
Could NOT find HIP: Found unsuitable version "1.5.18073", but required is
at least "1.5.18263" (found /opt/rocm)

For this question, let my sync some environment settings for yours.

Please type in following command to check the required hip version:

/opt/rocm/bin/hipconfig --version
/opt/rocm/hip/bin/hipconfig --version

Please check the print is "1.5.18263".

/opt/rocm/hip/lib/cmake/hip/hip-config-version.cmake
check the "set(PACKAGE_VERSION "1.5.18263")" if it is "1.5.18263"
For possible compatibility, please check is there multiple "hipconfig" or multiple hip-config-version.cmake install on your build system, in case with different version. I do think it 's because the build system has installed multiple version of rocm in the system

Conflicts: CMakeLists.txt

Xreki

LGTM

Conflicts: CMakeLists.txt

sabreshao and others added 3 commits October 10, 2018 07:27

add rocprim as cub library used by nv implementation

46cfe99

Signed-off-by: carlushuang <carlus.huang@amd.com> Conflicts: paddle/fluid/operators/CMakeLists.txt

Reduce build time in rocprim.

a4f5adf

carlushuang mentioned this pull request Oct 16, 2018

[AMDGPU] add platform/framework related code for amdgpu #13915

Closed

Merge branch 'develop' into amd_cmake_00

7f4516b

Conflicts: paddle/fluid/operators/CMakeLists.txt

Xreki added the AMD label Oct 31, 2018

Merge branch 'develop' into amd_cmake_00

661e365

Conflicts: CMakeLists.txt

Xreki reviewed Nov 2, 2018

View reviewed changes

Add rocprim introduction, remove useless cmake code.

cd00bf9

Merge branch 'develop' into amd_cmake_00

4077730

Conflicts: paddle/fluid/operators/CMakeLists.txt paddle/fluid/pybind/CMakeLists.txt paddle/testing/paddle_gtest_main.cc

Xreki reviewed Nov 22, 2018

View reviewed changes

Merge branch 'develop' into amd_cmake_00

28cdc85

Conflicts: CMakeLists.txt

Xreki approved these changes Nov 23, 2018

View reviewed changes

Xreki merged commit 61c5f13 into PaddlePaddle:develop Nov 23, 2018

sabreshao added 2 commits November 23, 2018 05:28

Merge branch 'develop' into amd_cmake_00

807064a

Conflicts: CMakeLists.txt

Remove useless flags and format cmake file.

9d73da8

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix cmake for AMDGPU platform #13801

Fix cmake for AMDGPU platform #13801

sabreshao commented Oct 10, 2018

sabreshao commented Oct 15, 2018

Xreki left a comment

Xreki Nov 2, 2018

sabreshao Nov 6, 2018

Xreki Nov 2, 2018

sabreshao Nov 6, 2018

Xreki Nov 2, 2018

sabreshao Nov 7, 2018

Xreki Nov 2, 2018

sabreshao Nov 7, 2018

Xreki Nov 2, 2018

sabreshao Nov 6, 2018

Xreki Nov 22, 2018

Xreki Nov 2, 2018

sabreshao Nov 6, 2018

Xreki Nov 2, 2018

sabreshao Nov 6, 2018

Xreki Nov 2, 2018

sabreshao Nov 6, 2018

sabreshao commented Nov 9, 2018

carlushuang commented Nov 22, 2018

Xreki left a comment

Xreki Nov 22, 2018

Xreki Nov 22, 2018

sabreshao Nov 22, 2018

Xreki Nov 23, 2018

sabreshao Nov 23, 2018

carlushuang commented Nov 22, 2018 •

edited

Loading

Xreki left a comment


		add_dependencies(rocprim extern_rocprim)

		LIST(APPEND external_project_dependencies rocprim)

Fix cmake for AMDGPU platform #13801

Fix cmake for AMDGPU platform #13801

Conversation

sabreshao commented Oct 10, 2018

sabreshao commented Oct 15, 2018

Xreki left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sabreshao commented Nov 9, 2018

carlushuang commented Nov 22, 2018

Xreki left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

carlushuang commented Nov 22, 2018 • edited Loading

Xreki left a comment

Choose a reason for hiding this comment

carlushuang commented Nov 22, 2018 •

edited

Loading