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

Remove dependency on platform threads library #7297

Merged
merged 7 commits into from
Jan 20, 2023
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
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
3 changes: 2 additions & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -726,7 +726,6 @@ HEADER_FILES = \
StrictifyFloat.h \
Substitute.h \
Target.h \
ThreadPool.h \
Tracing.h \
TrimNoOps.h \
Tuple.h \
Expand Down Expand Up @@ -2256,6 +2255,7 @@ install: $(LIB_DIR)/libHalide.a $(BIN_DIR)/libHalide.$(SHARED_EXT) $(INCLUDE_DIR
cp $(ROOT_DIR)/tools/halide_image_io.h $(PREFIX)/share/halide/tools
cp $(ROOT_DIR)/tools/halide_image_info.h $(PREFIX)/share/halide/tools
cp $(ROOT_DIR)/tools/halide_malloc_trace.h $(PREFIX)/share/halide/tools
cp $(ROOT_DIR)/tools/halide_thread_pool.h $(PREFIX)/share/halide/tools
ifeq ($(UNAME), Darwin)
install_name_tool -id $(PREFIX)/lib/libHalide.$(SHARED_EXT) $(PREFIX)/lib/libHalide.$(SHARED_EXT)
endif
Expand Down Expand Up @@ -2334,6 +2334,7 @@ $(DISTRIB_DIR)/lib/libHalide.$(SHARED_EXT): \
cp $(ROOT_DIR)/tools/halide_image_io.h $(DISTRIB_DIR)/tools
cp $(ROOT_DIR)/tools/halide_image_info.h $(DISTRIB_DIR)/tools
cp $(ROOT_DIR)/tools/halide_malloc_trace.h $(DISTRIB_DIR)/tools
cp $(ROOT_DIR)/tools/halide_thread_pool.h $(DISTRIB_DIR)/tools
cp $(ROOT_DIR)/tools/halide_trace_config.h $(DISTRIB_DIR)/tools
cp $(ROOT_DIR)/README*.md $(DISTRIB_DIR)
cp $(BUILD_DIR)/halide_config.* $(DISTRIB_DIR)
Expand Down
17 changes: 9 additions & 8 deletions README_cmake.md
Original file line number Diff line number Diff line change
Expand Up @@ -769,14 +769,15 @@ Variables set by the package:

Halide defines the following targets that are available to users:

| Imported target | Description |
|----------------------|--------------------------------------------------------------------------------------------------------------------------------------|
| `Halide::Halide` | this is the JIT-mode library to use when using Halide from C++. |
| `Halide::Generator` | this is the target to use when defining a generator executable. It supplies a `main()` function. |
| `Halide::Runtime` | adds include paths to the Halide runtime headers |
| `Halide::Tools` | adds include paths to the Halide tools, including the benchmarking utility. |
| `Halide::ImageIO` | adds include paths to the Halide image IO utility and sets up dependencies to PNG / JPEG if they are available. |
| `Halide::RunGenMain` | used with the `REGISTRATION` parameter of `add_halide_library` to create simple runners and benchmarking tools for Halide libraries. |
| Imported target | Description |
|----------------------|----------------------------------------------------------------------------------------------------------------------------------------------------------------------|
| `Halide::Halide` | this is the JIT-mode library to use when using Halide from C++. |
| `Halide::Generator` | this is the target to use when defining a generator executable. It supplies a `main()` function. |
| `Halide::Runtime` | adds include paths to the Halide runtime headers |
| `Halide::Tools` | adds include paths to the Halide tools, including the benchmarking utility. |
| `Halide::ImageIO` | adds include paths to the Halide image IO utility. Depends on `PNG::PNG` and `JPEG::JPEG` if they exist or were loaded through the corresponding package components. |
| `Halide::ThreadPool` | adds include paths to the Halide _simple_ thread pool utility library (this is not the same as the runtime's thread pool). Depends on `Threads::Threads`. |
alexreinking marked this conversation as resolved.
Show resolved Hide resolved
| `Halide::RunGenMain` | used with the `REGISTRATION` parameter of `add_halide_library` to create simple runners and benchmarking tools for Halide libraries. |

The following targets are not guaranteed to be available:

Expand Down
2 changes: 1 addition & 1 deletion cmake/HalideTestHelpers.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ if (NOT TARGET Halide::Test)
add_library(Halide_test INTERFACE)
add_library(Halide::Test ALIAS Halide_test)

# Obviously, link to the main library
# Obviously link to libHalide, but also grant all tests access to the threads library.
target_link_libraries(Halide_test INTERFACE Halide::Halide Threads::Threads)

# Everyone gets to see the common headers
Expand Down
2 changes: 1 addition & 1 deletion packaging/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ endforeach ()

target_sources(Halide_RunGenMain INTERFACE $<INSTALL_INTERFACE:${Halide_INSTALL_TOOLSDIR}/RunGenMain.cpp>)

install(TARGETS Halide_Tools Halide_ImageIO Halide_RunGenMain
install(TARGETS Halide_Tools Halide_ImageIO Halide_RunGenMain Halide_ThreadPool
EXPORT Halide_Interfaces
INCLUDES DESTINATION ${Halide_INSTALL_TOOLSDIR})

Expand Down
2 changes: 0 additions & 2 deletions src/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,6 @@ set(HEADER_FILES
StrictifyFloat.h
Substitute.h
Target.h
ThreadPool.h
Tracing.h
TrimNoOps.h
Tuple.h
Expand Down Expand Up @@ -378,7 +377,6 @@ add_library(Halide
$<TARGET_OBJECTS:Halide_initmod>)
add_library(Halide::Halide ALIAS Halide)

target_link_libraries(Halide PUBLIC Threads::Threads)
target_link_libraries(Halide PRIVATE Halide::LLVM)
target_link_libraries(Halide PUBLIC Halide::LanguageOptions)
target_compile_definitions(Halide PRIVATE $<$<STREQUAL:$<TARGET_PROPERTY:TYPE>,STATIC_LIBRARY>:Halide_STATIC_DEFINE>)
Expand Down
45 changes: 1 addition & 44 deletions src/Generator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
#include <condition_variable>
#include <fstream>
#include <memory>
#include <thread>
#include <unordered_map>
#include <utility>

Expand Down Expand Up @@ -896,49 +895,7 @@ gengen
user_error << o.str();
}

{
// TODO: should we move the TimeoutMonitor stuff to execute_generator?
// It seems more likely to be useful here.

struct TimeoutMonitor {
std::atomic<bool> generator_finished = false;
std::thread thread;
std::condition_variable cond_var;
std::mutex mutex;

// Kill the timeout monitor as a destructor to ensure the thread
// gets joined in the event of an exception
~TimeoutMonitor() {
generator_finished = true;
cond_var.notify_all();
thread.join();
}
} monitor;

const int timeout_in_seconds = std::stoi(flags_info["-t"]);
const auto timeout_time = std::chrono::steady_clock::now() + std::chrono::seconds(timeout_in_seconds);
monitor.thread = std::thread([timeout_time, timeout_in_seconds, &monitor]() {
std::unique_lock<std::mutex> lock(monitor.mutex);

if (timeout_in_seconds <= 0) {
// No watchdog timer, just let it run as long as it likes.
return;
}
while (!monitor.generator_finished) {
auto now = std::chrono::steady_clock::now();
if (now > timeout_time) {
fprintf(stderr, "Timed out waiting for Generator to complete (%d seconds)!\n", timeout_in_seconds);
fflush(stdout);
fflush(stderr);
exit(1);
} else {
monitor.cond_var.wait_for(lock, timeout_time - now);
}
}
});

execute_generator(args);
}
execute_generator(args);
return 0;
}

Expand Down
2 changes: 2 additions & 0 deletions src/autoschedulers/adams2019/AutoSchedule.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,8 @@
#ifdef _WIN32
#include <io.h>
#define _isatty isatty;
#else
#include <unistd.h>
#endif

namespace Halide {
Expand Down
5 changes: 4 additions & 1 deletion test/correctness/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -382,9 +382,12 @@ tests(GROUPS correctness multithreaded
unroll_huge_mux.cpp
)

# Make sure the test that needs image_io has it
# Make sure the test that needs Halide::ImageIO has it
target_link_libraries(correctness_image_io PRIVATE Halide::ImageIO)

# Make sure the test that needs Halide::ThreadPool has it
target_link_libraries(correctness_gpu_allocation_cache PRIVATE Halide::ThreadPool)

# Tests which use external funcs need to enable exports.
set_target_properties(correctness_async
correctness_atomics
Expand Down
3 changes: 2 additions & 1 deletion test/correctness/gpu_allocation_cache.cpp
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
#include "Halide.h"
#include "halide_benchmark.h"
#include "halide_thread_pool.h"

using namespace Halide;

Expand Down Expand Up @@ -136,7 +137,7 @@ int main(int argc, char **argv) {
// issues. Probably due to using the GL context on the wrong
// thread.
if (!target.has_feature(Target::OpenGLCompute)) {
Halide::Internal::ThreadPool<void> pool(1);
Halide::Tools::ThreadPool<void> pool(1);
std::vector<std::future<void>> futures;
futures.emplace_back(pool.async(test1, true));
futures.emplace_back(pool.async(test1, true));
Expand Down
10 changes: 8 additions & 2 deletions tools/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -35,16 +35,22 @@ target_include_directories(Halide_ImageIO INTERFACE $<BUILD_INTERFACE:${CMAKE_CU
add_library(Halide_RunGenMain INTERFACE)
add_library(Halide::RunGenMain ALIAS Halide_RunGenMain)
target_sources(Halide_RunGenMain INTERFACE $<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}/RunGenMain.cpp>)
target_link_libraries(Halide_RunGenMain INTERFACE Halide::Runtime Halide::ImageIO Threads::Threads)
target_link_libraries(Halide_RunGenMain INTERFACE Halide::Runtime Halide::ImageIO Halide::Tools)
set_target_properties(Halide_RunGenMain PROPERTIES EXPORT_NAME RunGenMain)

add_library(Halide_Generator INTERFACE)
add_library(Halide::Generator ALIAS Halide_Generator)
target_sources(Halide_Generator INTERFACE $<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}/GenGen.cpp>)
target_link_libraries(Halide_Generator INTERFACE Halide::Halide Threads::Threads ${CMAKE_DL_LIBS})
target_link_libraries(Halide_Generator INTERFACE Halide::Halide ${CMAKE_DL_LIBS})
set_target_properties(Halide_Generator PROPERTIES EXPORT_NAME Generator)

add_library(Halide_Tools INTERFACE)
add_library(Halide::Tools ALIAS Halide_Tools)
target_include_directories(Halide_Tools INTERFACE $<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}>)
set_target_properties(Halide_Tools PROPERTIES EXPORT_NAME Tools)

add_library(Halide_ThreadPool INTERFACE)
add_library(Halide::ThreadPool ALIAS Halide_ThreadPool)
target_link_libraries(Halide_ThreadPool INTERFACE Threads::Threads)
target_include_directories(Halide_Tools INTERFACE $<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}>)
set_target_properties(Halide_ThreadPool PROPERTIES EXPORT_NAME ThreadPool)
5 changes: 3 additions & 2 deletions src/ThreadPool.h → tools/halide_thread_pool.h
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
#ifndef HALIDE_THREAD_POOL_H
#define HALIDE_THREAD_POOL_H

#include <cassert>
#include <condition_variable>
#include <functional>
#include <future>
Expand Down Expand Up @@ -35,7 +36,7 @@
* *not* intended to be the underlying implementation for Halide runtime threads
*/
namespace Halide {
namespace Internal {
namespace Tools {

template<typename T>
class ThreadPool {
Expand Down Expand Up @@ -154,7 +155,7 @@ inline void ThreadPool<void>::Job::run_unlocked(std::unique_lock<std::mutex> &un
result.set_value();
}

} // namespace Internal
} // namespace Tools
} // namespace Halide

#endif // HALIDE_THREAD_POOL_H