Add CMake for documentation #442
Conversation
matyas-streamhpc
force-pushed
the
add-cmake-doc
branch
from
51ba9aa
to
5d5ff41
Compare
|
@feizheng10 @malcolmroberts Can I get a review for this PR? |
CMakeLists.txt
Outdated
| @@ -137,6 +139,9 @@ if(BUILD_FILE_REORG_BACKWARD_COMPATIBILITY AND NOT WIN32) | |||
| ) | |||
| endif() | |||
|
|
|||
| # Enable documentation build | |||
| option(BUILD_DOCS "Build documentation" ON) | |||
There was a problem hiding this comment.
I don't recall the standard... but all rocFFT specific build option should start with ROCFFT_, do we? Unless it is a rocm cmake level stuff.
There was a problem hiding this comment.
I would think we want to make similar changes to all math libs, not just rocFFT. So IMO the generic name makes more sense.
There was a problem hiding this comment.
This will be a ROCm wide build option.
There was a problem hiding this comment.
BUILD_DOCS should be OFF by default
|
The general idea here is definitely good - when I think ahead to wanting to making a similar change for all math libs, I'm wondering whether at least some of this should be moved into rocm-cmake so it doesn't have to be redone for each library. The root CMakeLists.txt also is prepared to fetch rocm-cmake, so doing it again in the docs dir seems redundant. @lawruble13, can you think of a better way to do this? |
cmake/Dependencies.cmake
Outdated
| rocm-cmake | ||
| GIT_REPOSITORY https://github.com/RadeonOpenCompute/rocm-cmake.git | ||
| GIT_TAG ${rocm_cmake_tag} | ||
| SOURCE_SUBDIR "DISABLE ADDING TO BUILD" # We don't really want to consume the build and test targets of ROCm CMake. |
There was a problem hiding this comment.
I suppose we should consider doing something to better support this usage in rocm-cmake. Any suggestions?
There was a problem hiding this comment.
Let's standardize all rocm projects to have this cmake/Dependencies file that will download rocm-cmake.
CMakeLists.txt
Outdated
| @@ -137,6 +139,9 @@ if(BUILD_FILE_REORG_BACKWARD_COMPATIBILITY AND NOT WIN32) | |||
| ) | |||
| endif() | |||
|
|
|||
| # Enable documentation build | |||
| option(BUILD_DOCS "Build documentation" ON) | |||
There was a problem hiding this comment.
BUILD_DOCS should be OFF by default
CMakeLists.txt
Outdated
| @@ -47,6 +47,8 @@ endif() | |||
|
|
|||
| set( ROCFFT_BUILD_SCOPE ON ) | |||
|
|
|||
| include(GNUInstallDirs) | |||
|
|
|||
| project( rocfft LANGUAGES CXX C ) | |||
|
|
|||
| # This finds the rocm-cmake project, and installs it if not found | |||
There was a problem hiding this comment.
We need to have only one place that rocm-cmake is downloaded
matyas-streamhpc
force-pushed
the
add-cmake-doc
branch
from
aac8e1b
to
058d3df
Compare
matyas-streamhpc
force-pushed
the
add-cmake-doc
branch
from
44a71ec
to
5a40b79
Compare
cmake/Dependencies.cmake
Outdated
| if(NOT ROCM_FOUND) | ||
| message(STATUS "ROCm CMake not found. Fetching...") | ||
| set(rocm_cmake_tag | ||
| "c044bb52ba85058d28afe2313be98d9fed02e293" # [email protected]. (move to 6.0 tag when released) |
matyas-streamhpc
requested review from
af-ayala and
eng-flavio-teixeira
as code owners
matyas-streamhpc
force-pushed
the
add-cmake-doc
branch
from
ce449c9
to
8b1716e
Compare
|
@malcolmroberts shall I merge the PR into this repository? |
matyas-streamhpc
force-pushed
the
add-cmake-doc
branch
from
8b1716e
to
c44b300
Compare
|
Rebased. |
matyas-streamhpc
force-pushed
the
add-cmake-doc
branch
from
c44b300
to
38c71ae
Compare
|
Rebased. |
Summary of proposed changes:
Add CMake support for documentation.