Skip to content

Don't include sources for disabled targets #5503

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
JanHett wants to merge 2 commits into from
Closed

Don't include sources for disabled targets #5503

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
38711f2
Configure source files and headers based on set options
JanHett Dec 1, 2020
a9b1905
Exclude target-specific code based on CMake defines
JanHett Dec 1, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
249 changes: 193 additions & 56 deletions src/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,165 @@
##
# Lists of source files. Keep ALL lists sorted in alphabetical order.
# Set up additional backend options for Halide
##
option(TARGET_OPENCL "Include OpenCL-C target" ON)
option(TARGET_OPENGL "Include OpenGL/GLSL target" ON)
option(TARGET_METAL "Include Metal target" ON)
option(TARGET_D3D12COMPUTE "Include Direct3D 12 Compute target" ON)

##
# List of optional targets
##

set(OPTIONAL_TARGETS
Copy link
Member

@alexreinking alexreinking Dec 1, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where possible, I would like to enforce the convention that ALL_CAPS variables are reserved for cache variables and function arguments. Normal variables like optional_targets here should be lower_snake_case.

AARCH64
AMDGPU
ARM
HEXAGON
MIPS
NVPTX
POWERPC
RISCV
WEBASSEMBLY
X86
OPENCL
OPENGL
METAL
D3D12COMPUTE
)

##
# Lists of target-specific source files.
# Keep ALL lists sorted in alphabetical order.
##

##
# Header files
##

# The externally-visible header files that go into making Halide.h.
# Don't include anything here that includes llvm headers.

set(AARCH64_HEADER_FILES
)

set(AMDGPU_HEADER_FILES
)

set(ARM_HEADER_FILES
CodeGen_ARM.h
)

set(HEXAGON_HEADER_FILES
HexagonOffload.h
HexagonOptimize.h
)

set(MIPS_HEADER_FILES
CodeGen_MIPS.h
)

set(NVPTX_HEADER_FILES
CodeGen_PTX_Dev.h
)

set(POWERPC_HEADER_FILES
CodeGen_PowerPC.h
)

set(RISCV_HEADER_FILES
CodeGen_RISCV.h
)

set(WEBASSEMBLY_HEADER_FILES
CodeGen_WebAssembly.h
)

set(X86_HEADER_FILES
CodeGen_X86.h
)

set(OPENCL_HEADER_FILES
CodeGen_OpenCL_Dev.h
)

set(OPENGL_HEADER_FILES
CodeGen_OpenGL_Dev.h
CodeGen_OpenGLCompute_Dev.h
InjectOpenGLIntrinsics.h
)

set(METAL_HEADER_FILES
CodeGen_Metal_Dev.h
)

set(D3D12COMPUTE_HEADER_FILES
CodeGen_D3D12Compute_Dev.h
)

##
# Source files
##
set(AARCH64_SOURCE_FILES
)

set(AMDGPU_SOURCE_FILES
)

Comment on lines +102 to +107
Copy link
Member

@alexreinking alexreinking Dec 1, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need to have this span multiple lines. Better to show that it's meant to be empty via set(AARCH64_SOURCE_FILES "")

set(ARM_SOURCE_FILES
CodeGen_ARM.cpp
)

set(HEXAGON_SOURCE_FILES
CodeGen_Hexagon.cpp
HexagonOffload.cpp
HexagonOptimize.cpp
)

set(MIPS_SOURCE_FILES
CodeGen_MIPS.cpp
)

set(NVPTX_SOURCE_FILES
CodeGen_PTX_Dev.cpp
)

set(POWERPC_SOURCE_FILES
CodeGen_PowerPC.cpp
)

set(RISCV_SOURCE_FILES
CodeGen_RISCV.cpp
)

set(WEBASSEMBLY_SOURCE_FILES
CodeGen_WebAssembly.cpp
)

set(X86_SOURCE_FILES
CodeGen_X86.cpp
)

set(OPENCL_SOURCE_FILES
CodeGen_OpenCL_Dev.cpp
)

set(OPENGL_SOURCE_FILES
CodeGen_OpenGL_Dev.cpp
CodeGen_OpenGLCompute_Dev.cpp
InjectOpenGLIntrinsics.cpp
)

set(METAL_SOURCE_FILES
CodeGen_Metal_Dev.cpp
)

set(D3D12COMPUTE_SOURCE_FILES
CodeGen_D3D12Compute_Dev.cpp
)

##
# Lists of non-optional source files.
# Keep ALL lists sorted in alphabetical order.
##

# The externally-visible header files that go into making Halide.h.
Expand All @@ -23,25 +183,13 @@ set(HEADER_FILES
Buffer.h
CanonicalizeGPUVars.h
Closure.h
CodeGen_ARM.h
CodeGen_C.h
CodeGen_D3D12Compute_Dev.h
CodeGen_GPU_Dev.h
CodeGen_GPU_Host.h
CodeGen_Internal.h
CodeGen_LLVM.h
CodeGen_Metal_Dev.h
CodeGen_MIPS.h
CodeGen_OpenCL_Dev.h
CodeGen_OpenGL_Dev.h
CodeGen_OpenGLCompute_Dev.h
CodeGen_Posix.h
CodeGen_PowerPC.h
CodeGen_PTX_Dev.h
CodeGen_PyTorch.h
CodeGen_RISCV.h
CodeGen_WebAssembly.h
CodeGen_X86.h
CompilerLogger.h
ConciseCasts.h
CPlusPlusMangle.h
Expand Down Expand Up @@ -76,12 +224,9 @@ set(HEADER_FILES
FuseGPUThreadLoops.h
FuzzFloatStores.h
Generator.h
HexagonOffload.h
HexagonOptimize.h
ImageParam.h
InferArguments.h
InjectHostDevBufferCopies.h
InjectOpenGLIntrinsics.h
Inline.h
InlineReductions.h
IntegerDivisionTable.h
Expand Down Expand Up @@ -188,26 +333,13 @@ set(SOURCE_FILES
Buffer.cpp
CanonicalizeGPUVars.cpp
Closure.cpp
CodeGen_ARM.cpp
CodeGen_C.cpp
CodeGen_D3D12Compute_Dev.cpp
CodeGen_GPU_Dev.cpp
CodeGen_GPU_Host.cpp
CodeGen_Hexagon.cpp
CodeGen_Internal.cpp
CodeGen_LLVM.cpp
CodeGen_Metal_Dev.cpp
CodeGen_MIPS.cpp
CodeGen_OpenCL_Dev.cpp
CodeGen_OpenGL_Dev.cpp
CodeGen_OpenGLCompute_Dev.cpp
CodeGen_Posix.cpp
CodeGen_PowerPC.cpp
CodeGen_PTX_Dev.cpp
CodeGen_PyTorch.cpp
CodeGen_RISCV.cpp
CodeGen_WebAssembly.cpp
CodeGen_X86.cpp
CompilerLogger.cpp
CPlusPlusMangle.cpp
CSE.cpp
Expand Down Expand Up @@ -236,12 +368,9 @@ set(SOURCE_FILES
FuseGPUThreadLoops.cpp
FuzzFloatStores.cpp
Generator.cpp
HexagonOffload.cpp
HexagonOptimize.cpp
ImageParam.cpp
InferArguments.cpp
InjectHostDevBufferCopies.cpp
InjectOpenGLIntrinsics.cpp
Inline.cpp
InlineReductions.cpp
IntegerDivisionTable.cpp
Expand Down Expand Up @@ -341,6 +470,28 @@ set(SOURCE_FILES
WrapCalls.cpp
)

##
# Create optional targets
##

foreach(OPTIONAL_TARGET ${OPTIONAL_TARGETS})
Copy link
Member

@alexreinking alexreinking Dec 1, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use modern cmake... foreach (optional_target IN LISTS optional_targets).

if(TARGET_${OPTIONAL_TARGET})
add_library(Halide_${OPTIONAL_TARGET} INTERFACE)
target_sources(Halide_${OPTIONAL_TARGET} INTERFACE
${${OPTIONAL_TARGET}_SOURCE_FILES}
)
Comment on lines +481 to +482
Copy link
Member

@alexreinking alexreinking Dec 1, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add ${${OPTIONAL_TARGET}_HEADER_FILES}

target_compile_definitions(Halide_${OPTIONAL_TARGET} INTERFACE
WITH_${OPTIONAL_TARGET})

list(APPEND HEADER_FILES ${${OPTIONAL_TARGET}_HEADER_FILES})
Copy link
Member

@alexreinking alexreinking Dec 1, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need for this line (see above)

endif()
endforeach()

# Special case definition for D3D12COMPUTE for compatibility
if (TARGET_D3D12COMPUTE)
target_compile_definitions(Halide_D3D12COMPUTE INTERFACE WITH_D3D12)
endif ()

##
# Build and import the runtime.
##
Expand Down Expand Up @@ -373,6 +524,16 @@ add_library(Halide
$<TARGET_OBJECTS:Halide_initmod>)
add_library(Halide::Halide ALIAS Halide)

##
# Link optional targets to the main library
##

foreach(OPTIONAL_TARGET ${OPTIONAL_TARGETS})
Copy link
Member

@alexreinking alexreinking Dec 1, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use modern cmake... foreach (optional_target IN LISTS optional_targets).

target_link_libraries(Halide PRIVATE
$<BUILD_INTERFACE:$<TARGET_NAME_IF_EXISTS:Halide_${OPTIONAL_TARGET}>>
)
endforeach()

target_link_libraries(Halide PRIVATE Halide::LLVM)
target_link_libraries(Halide PUBLIC Halide::LanguageOptions)
target_compile_definitions(Halide
Expand Down Expand Up @@ -445,30 +606,6 @@ target_compile_definitions(Halide
$<$<CXX_COMPILER_ID:MSVC>:_SCL_SECURE_NO_WARNINGS>
)

##
# Set up additional backend options for Halide
##

option(TARGET_OPENCL "Include OpenCL-C target" ON)
if (TARGET_OPENCL)
target_compile_definitions(Halide PRIVATE WITH_OPENCL)
endif ()

option(TARGET_OPENGL "Include OpenGL/GLSL target" ON)
if (TARGET_OPENGL)
target_compile_definitions(Halide PRIVATE WITH_OPENGL)
endif ()

option(TARGET_METAL "Include Metal target" ON)
if (TARGET_METAL)
target_compile_definitions(Halide PRIVATE WITH_METAL)
endif ()

option(TARGET_D3D12COMPUTE "Include Direct3D 12 Compute target" ON)
if (TARGET_D3D12COMPUTE)
target_compile_definitions(Halide PRIVATE WITH_D3D12)
endif ()

##
# Add autoschedulers to the build.
##
Expand Down
30 changes: 24 additions & 6 deletions src/CodeGen_GPU_Host.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,13 +1,23 @@
#include <sstream>

#include "CodeGen_D3D12Compute_Dev.h"
#ifdef WITH_D3D12
Copy link
Member

@alexreinking alexreinking Dec 1, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would be better to move these into the header guards so that they may be included unconditionally.

#include "CodeGen_D3D12Compute_Dev.h"
#endif
#include "CodeGen_GPU_Host.h"
#include "CodeGen_Internal.h"
#include "CodeGen_Metal_Dev.h"
#include "CodeGen_OpenCL_Dev.h"
#include "CodeGen_OpenGLCompute_Dev.h"
#include "CodeGen_OpenGL_Dev.h"
#include "CodeGen_PTX_Dev.h"
#ifdef WITH_METAL
#include "CodeGen_Metal_Dev.h"
#endif
#ifdef WITH_OPENCL
#include "CodeGen_OpenCL_Dev.h"
#endif
#ifdef WITH_OPENGL
#include "CodeGen_OpenGLCompute_Dev.h"
#include "CodeGen_OpenGL_Dev.h"
#endif
#ifdef WITH_NVPTX
#include "CodeGen_PTX_Dev.h"
#endif
#include "Debug.h"
#include "DeviceArgument.h"
#include "ExprUsesVar.h"
Expand Down Expand Up @@ -102,6 +112,7 @@ CodeGen_GPU_Host<CodeGen_CPU>::CodeGen_GPU_Host(Target target)
// OpenCL, CUDA, OpenGLCompute, and OpenGL last.
// The code is in reverse order to allow later tests to override
// earlier ones.
#ifdef WITH_OPENGL
Copy link
Member

@alexreinking alexreinking Dec 1, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should probably not be skipping the runtime check for the target feature, but instead throwing an error that Halide was not compiled with support for the requested backend.

if (target.has_feature(Target::OpenGL)) {
debug(1) << "Constructing OpenGL device codegen\n";
cgdev[DeviceAPI::GLSL] = new CodeGen_OpenGL_Dev(target);
Expand All @@ -110,22 +121,29 @@ CodeGen_GPU_Host<CodeGen_CPU>::CodeGen_GPU_Host(Target target)
debug(1) << "Constructing OpenGL Compute device codegen\n";
cgdev[DeviceAPI::OpenGLCompute] = new CodeGen_OpenGLCompute_Dev(target);
}
#endif
if (target.has_feature(Target::CUDA)) {
debug(1) << "Constructing CUDA device codegen\n";
cgdev[DeviceAPI::CUDA] = new CodeGen_PTX_Dev(target);
}
#ifdef WITH_OPENCL
if (target.has_feature(Target::OpenCL)) {
debug(1) << "Constructing OpenCL device codegen\n";
cgdev[DeviceAPI::OpenCL] = new CodeGen_OpenCL_Dev(target);
}
#endif
#ifdef WITH_METAL
if (target.has_feature(Target::Metal)) {
debug(1) << "Constructing Metal device codegen\n";
cgdev[DeviceAPI::Metal] = new CodeGen_Metal_Dev(target);
}
#endif // WITH_METAL
#ifdef WITH_D3D12
if (target.has_feature(Target::D3D12Compute)) {
debug(1) << "Constructing Direct3D 12 Compute device codegen\n";
cgdev[DeviceAPI::D3D12Compute] = new CodeGen_D3D12Compute_Dev(target);
}
#endif // WITH_D3D12

if (cgdev.empty()) {
internal_error << "Requested unknown GPU target: " << target.to_string() << "\n";
Expand Down
Loading