Skip to content
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.

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

Add CMake build rules for (some) of the generator jit tests #7693

Open
wants to merge 15 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 10 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
13 changes: 10 additions & 3 deletions cmake/HalideGeneratorHelpers.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -92,9 +92,15 @@ function(add_halide_generator TARGET)

# TODO: what do we need to do for PACKAGE_NAME PACKAGE_NAMESPACE EXPORT_FILE in this case?
else ()
add_executable(${TARGET} ${ARG_SOURCES})
# Make a library of the Generator that can be used for (e.g.) cpp_stub.
add_library("${TARGET}_lib" OBJECT ${ARG_SOURCES})
target_link_libraries("${TARGET}_lib" PRIVATE Halide::Halide ${ARG_LINK_LIBRARIES})
target_include_directories("${TARGET}_lib" INTERFACE "$<BUILD_INTERFACE:${CMAKE_CURRENT_BINARY_DIR}>")

add_executable(${TARGET} "$<TARGET_OBJECTS:${TARGET}_lib>")
target_link_libraries("${TARGET}" PRIVATE Halide::Generator)

steven-johnson marked this conversation as resolved.
Show resolved Hide resolved
add_executable(${gen} ALIAS ${TARGET})
target_link_libraries(${TARGET} PRIVATE Halide::Generator ${ARG_LINK_LIBRARIES})

if (NOT ARG_NO_DEFAULT_FLAGS AND NOT Halide_NO_DEFAULT_FLAGS)
# For crosscompiling builds, the Halide headers will be included using -isystem,
Expand Down Expand Up @@ -182,11 +188,11 @@ function(add_halide_library TARGET)
# - `c_source` is selected by C_BACKEND
# - `object` is selected for CMake-target-compile
# - `static_library` is selected for cross-compile
# - `cpp_stub` is not available
set(extra_output_names
ASSEMBLY
BITCODE
COMPILER_LOG
CPP_STUB
FEATURIZATION
FUNCTION_INFO_HEADER
LLVM_ASSEMBLY
Expand All @@ -201,6 +207,7 @@ function(add_halide_library TARGET)
set(ASSEMBLY_extension ".s")
set(BITCODE_extension ".bc")
set(COMPILER_LOG_extension ".halide_compiler_log")
set(CPP_STUB_extension ".stub.h")
set(FEATURIZATION_extension ".featurization")
set(FUNCTION_INFO_HEADER_extension ".function_info.h")
set(LLVM_ASSEMBLY_extension ".ll")
Expand Down
1 change: 1 addition & 0 deletions src/Generator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1630,6 +1630,7 @@ bool GeneratorBase::emit_cpp_stub(const std::string &stub_file_path) {
GeneratorParamInfo &pi = param_info();
std::ofstream file(stub_file_path);
StubEmitter emit(file, generator_registered_name, generator_stub_name, pi.generator_params(), pi.inputs(), pi.outputs());
debug(1) << "GeneratorBase::emit_cpp_stub(): generating cpp_stub at " << stub_file_path << "\n";
emit.emit();
return true;
}
Expand Down
43 changes: 37 additions & 6 deletions test/generator/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ function(_add_halide_libraries TARGET)
FEATURES "${args_FEATURES}"
PARAMS "${args_PARAMS}"
PLUGINS "${args_PLUGINS}"
CPP_STUB cpp_stub_out
Copy link
Member

Choose a reason for hiding this comment

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

This is unused but really shouldn't be... which target consumes it? It should be listed in the target_sources of that target.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hrm, ok.

I'm also baffled by the build failures in apps/hannk -- something has somehow tickled the include paths in a way that makes no obvious sense to me.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is unused but really shouldn't be... which target consumes it? It should be listed in the target_sources of that target.

It's just a header file -- do those need to be in target_sources in CMake?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ping re "It's just a header file -- do those need to be in target_sources in CMake?"

Copy link
Member

Choose a reason for hiding this comment

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

It's just a header file -- do those need to be in target_sources in CMake?

If the header file is generated, then it should be listed in some target, yes. Either:

  1. Directly among the PRIVATE sources of the unique target that consumes it.
  2. As a DEPENDS of a custom target; other targets that use the header should add_dependencies to that target.

FUNCTION_INFO_HEADER function_info_header_out)
if (args_EXTERNS)
target_link_libraries(${TARGET} INTERFACE ${args_EXTERNS})
Expand Down Expand Up @@ -229,12 +230,6 @@ endif ()
_add_halide_libraries(acquire_release)
_add_halide_aot_tests(acquire_release)

# TODO: what are these?
# configure_jittest.cpp
# example_jittest.cpp
# registration_test.cpp
# rungen_test.cpp

# alias_aottest.cpp
# alias_generator.cpp
set(EXTRA_ALIAS_LIBS alias_with_offset_42 alias_Adams2019 alias_Li2018 alias_Mullapudi2016)
Expand Down Expand Up @@ -622,3 +617,39 @@ _add_halide_aot_tests(variable_num_threads
# Requires threading support, not yet available for wasm tests
ENABLE_IF NOT ${_USING_WASM}
GROUPS multithreaded)


##
# Create targets for the JIT tests
##

function(_add_halide_jit_test NAME)
set(options "")
set(oneValueArgs "")
set(multiValueArgs GROUPS)
cmake_parse_arguments(args "${options}" "${oneValueArgs}" "${multiValueArgs}" ${ARGN})

set(TARGET "generator_jit_${NAME}")
set(SRCS "${NAME}_jittest.cpp")

add_executable("${TARGET}" "${SRCS}")
target_link_libraries("${TARGET}" PRIVATE "${NAME}.generator_lib" Halide::Test Halide::TerminateHandler)
steven-johnson marked this conversation as resolved.
Show resolved Hide resolved

# Some of the jit tests require the cpp_stub file from the Generator, so ensure it's built first:
add_dependencies("${TARGET}" "${NAME}")

add_halide_test(${TARGET} GROUPS generator ${args_GROUPS})

endfunction()

# configure_jittest.cpp
# configure_generator.cpp
_add_halide_jit_test(configure)

# example_jittest.cpp
# example_generator.cpp
_add_halide_jit_test(example GROUPS multithreaded)

# TODO:
# registration_test.cpp
# rungen_test.cpp
8 changes: 7 additions & 1 deletion test/generator/configure_jittest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,13 @@ void verify(const Buffer<int32_t, 3> &img, float compiletime_factor, float runti
}

int main(int argc, char **argv) {
GeneratorContext context(get_jit_target_from_environment());
Target t = get_jit_target_from_environment();
if (t.has_feature(Target::WebGPU) || t.has_feature(Target::WasmThreads)) {
printf("[SKIP] This test does not support WebGPU or WasmThreads.\n");
return 0;
}

GeneratorContext context(t);

Buffer<int, 3> input(kSize, kSize, 3);
input.for_each_element([&](int x, int y, int c) {
Expand Down
9 changes: 8 additions & 1 deletion test/generator/example_jittest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,14 @@ void verify(const Buffer<int32_t, 3> &img, float compiletime_factor, float runti
}

int main(int argc, char **argv) {
GeneratorContext context(get_jit_target_from_environment());
Target t = get_jit_target_from_environment();
if (t.has_feature(Target::WebGPU) || t.has_feature(Target::WasmThreads)) {
printf("[SKIP] This test does not support WebGPU or WasmThreads.\n");
return 0;
}

GeneratorContext context(t);

const float runtime_factor = 4.5f;

// Demonstrate (and test) various ways to use a Stub to invoke a Generator with the JIT.
Expand Down