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.

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

Jump to bottom

Fixes to avoid multiple definitions for thread pools and suppress compiler warnings. #1

Merged
merged 7 commits into from
Jan 6, 2022
Merged

Fixes to avoid multiple definitions for thread pools and suppress compiler warnings. #1

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
19821db
Fix problems with multiple definitions for non-inlined functions.
cheshyre Aug 31, 2021
5bd212a
Suppress pedantic ISO C++ warning about __int128.
cheshyre Aug 31, 2021
8129913
Perform (hopefully safe) static cast to signed integer before compare.
cheshyre Aug 31, 2021
d175ff7
Add explicit no-ops to suppress unused variable warnings.
cheshyre Aug 31, 2021
4e8331b
Switch atomic type to __uint128_t as suggested by @Morwenn.
cheshyre Jan 4, 2022
aa36a23
Change cast of std::size_t from sizeof to int instead of int64_t.
cheshyre Jan 4, 2022
59e2400
Remove unused variables and function parameters.
cheshyre Jan 4, 2022
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
4 changes: 3 additions & 1 deletion include/ips4o/bucket_pointers.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,9 @@ class Sorter<Cfg>::BucketPointers {

#elif defined(__SIZEOF_INT128__)

using atomic_type = unsigned __int128;
// __extension__ suppresses the pedantic warning about __int128
// not being ISO C++ compliant
__extension__ using atomic_type = unsigned __int128;
Copy link

Choose a reason for hiding this comment

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

Unless I'm mistaken, you can also use __uint128_t to avoid the pedantic warning without having to use __extension__.

Copy link
Author

Choose a reason for hiding this comment

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

It seems that it does, and a quick Google suggests that it supported by at least Clang, GCC, and Intel's compiler. I don't know enough about compilers and support to say for certain that this is a safe replacement, but if it is then it might be preferable to __extension__.

Copy link

Choose a reason for hiding this comment

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

I'm unfortunately no more knowledgeable, I just know that the use of __int128_t and __uint128_t is somewhat widespread and have the advantage of being "normal" from the language POV, just another type which happens to have compiler support.


#endif // defined( __SIZEOF_INT128__)

Expand Down
3 changes: 2 additions & 1 deletion include/ips4o/classifier.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@

#pragma once

#include <cstdint>
#include <type_traits>
#include <utility>

Expand Down Expand Up @@ -126,7 +127,7 @@ class Sorter<Cfg>::Classifier {
template <bool kEqualBuckets, class Yield, int...Args>
void classifySwitch(iterator begin, iterator end, Yield&& yield,
std::integer_sequence<int, Args...>) const {
IPS4OML_ASSUME_NOT(log_buckets_ <= 0 && log_buckets_ >= sizeof...(Args));
IPS4OML_ASSUME_NOT(log_buckets_ <= 0 && log_buckets_ >= static_cast<std::int64_t>(sizeof...(Args)));
((Args == log_buckets_ &&
classifyUnrolled<kEqualBuckets, Args>(begin, end, std::forward<Yield>(yield)))
|| ...);
Expand Down
4 changes: 4 additions & 0 deletions include/ips4o/parallel.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ namespace detail {
*/
template <class Cfg>
void Sorter<Cfg>::processSmallTasks(const iterator begin, int num_threads) {
(void)num_threads;
auto& scheduler = shared_->scheduler;
auto& my_queue = local_.seq_task_queue;
Task task;
Expand Down Expand Up @@ -185,6 +186,7 @@ void Sorter<Cfg>::setShared(SharedData* shared) {
*/
template <class Cfg>
void Sorter<Cfg>::processBigTasksSecondary(const int id, BufferStorage& buffer_storage) {
(void)buffer_storage;
BigTask& task = shared_->big_tasks[id];
auto partial_thread_pool = shared_->thread_pools[task.root_thread];

Expand Down Expand Up @@ -281,6 +283,7 @@ std::pair<std::vector<typename Cfg::difference_type>, bool>
Sorter<Cfg>::parallelPartitionPrimary(const iterator begin, const iterator end,
const int num_threads) {
const auto size = end - begin;
(void)size;

const auto res = partition<true>(begin, end, shared_->bucket_start, 0, num_threads);
const int num_buckets = std::get<0>(res);
Expand Down Expand Up @@ -395,6 +398,7 @@ class ParallelSorter {

// Set up base data before switching to parallel mode
auto& shared = shared_ptr_.get();
(void)shared;

// Execute in parallel
thread_pool_(
Expand Down
18 changes: 9 additions & 9 deletions include/ips4o/thread_pool.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,7 @@ class StdThreadPool {
/**
* Constructor for the std::thread pool.
*/
StdThreadPool::Impl::Impl(int num_threads)
inline StdThreadPool::Impl::Impl(int num_threads)
: sync_(std::max(1, num_threads))
, pool_barrier_(std::max(1, num_threads))
, num_threads_(num_threads)
Expand All @@ -184,7 +184,7 @@ StdThreadPool::Impl::Impl(int num_threads)
/**
* Destructor for the std::thread pool.
*/
StdThreadPool::Impl::~Impl() {
inline StdThreadPool::Impl::~Impl() {
done_ = true;
pool_barrier_.barrier();
for (auto& t : threads_)
Expand All @@ -195,7 +195,7 @@ StdThreadPool::Impl::~Impl() {
* Entry point for parallel execution for the std::thread pool.
*/
template <class F>
void StdThreadPool::Impl::run(F&& func, int num_threads) {
inline void StdThreadPool::Impl::run(F&& func, int num_threads) {
func_ = func;
num_threads_ = num_threads;
sync_.setNumThreads(num_threads);
Expand All @@ -208,7 +208,7 @@ void StdThreadPool::Impl::run(F&& func, int num_threads) {
/**
* Main loop for threads created by the std::thread pool.
*/
void StdThreadPool::Impl::main(const int my_id) {
inline void StdThreadPool::Impl::main(const int my_id) {
for (;;) {
pool_barrier_.barrier();
if (done_) break;
Expand Down Expand Up @@ -276,13 +276,13 @@ class ThreadJoiningThreadPool {
/**
* Constructor for the std::thread pool.
*/
ThreadJoiningThreadPool::Impl::Impl(int num_threads)
inline ThreadJoiningThreadPool::Impl::Impl(int num_threads)
: sync_(num_threads), pool_barrier_(num_threads), num_threads_(num_threads) {}

/**
* Destructor for the std::thread pool.
*/
ThreadJoiningThreadPool::Impl::~Impl() { assert(done_ == true); }
inline ThreadJoiningThreadPool::Impl::~Impl() { assert(done_ == true); }

/**
* Entry point for parallel execution for the std::thread pool.
Expand All @@ -301,7 +301,7 @@ void ThreadJoiningThreadPool::Impl::run(F&& func, int num_threads) {
/**
* Main loop for threads which have joined.
*/
void ThreadJoiningThreadPool::Impl::main(const int my_id) {
inline void ThreadJoiningThreadPool::Impl::main(const int my_id) {
for (;;) {
pool_barrier_.barrier();
if (done_) break;
Expand All @@ -311,9 +311,9 @@ void ThreadJoiningThreadPool::Impl::main(const int my_id) {
}
}

void ThreadJoiningThreadPool::Impl::join(int my_id) { main(my_id); }
inline void ThreadJoiningThreadPool::Impl::join(int my_id) { main(my_id); }

void ThreadJoiningThreadPool::Impl::release_threads() {
inline void ThreadJoiningThreadPool::Impl::release_threads() {
done_ = true;
pool_barrier_.barrier();
}
Expand Down