bshoshany / thread-pool

BS::thread_pool: a fast, lightweight, and easy-to-use C++17 thread pool library
MIT License
2.21k stars 253 forks source link

[BUG] #123

Closed PengBo410 closed 1 year ago

PengBo410 commented 1 year ago

Describe the bug Hello, first of all, thank you for your project. Both its performance and detailed annotations have greatly benefited me. However, during a simple test, I encountered a compilation error. I am not sure if this error could be helpful for the improvement of the project (please ignore it if it is due to my misunderstanding). Thank you!

Minimal working example

// Copy the following code into a new cpp file and name it: test.cpp
#include <iostream>
#include <assert.h>
#include <omp.h>
#include "BS_thread_pool.hpp"

int main(int argc, char* argv[])
{
    constexpr int label = 1;

    uint64_t num = 1e9;
    uint64_t totalSum = 0;
    BS::timer time;

    if constexpr (label == 0)
    {       
        BS::thread_pool myPool(12);       
        auto addF = [num](const int64_t start, const int64_t end)
        {
            uint64_t sum = 0;
            for(size_t i = start; i< end; i++)
            {
                sum += i;
            }
            return sum;
        };
        time.start();
        const std::vector<uint64_t> sum_vec = myPool.parallelize_loop(num, addF).get();
        for (const uint64_t& sum : sum_vec)
        {
            totalSum += sum;
        }
        time.stop();
        std::cout << "BS::thread_pool time: " << time.ms() << " (ms)" << std::endl;
    }
    else if constexpr (label == 1)
    {      
        time.start();
        #pragma omp parallel for num_threads(12) reduction(+: totalSum)
        for (size_t i = 0; i < num; i++)
        {
            totalSum += i;
        }
        time.stop();
        std::cout << "OMP time: " << time.ms() << " (ms)" << std::endl;
    }
    else
    {
        std::cout << "Error label" << std::endl;
    }

    //check
    uint64_t totalSum_check = 0;
    for (size_t i = 0; i < num; i++)
    {
       totalSum_check += i;
    }
    assert(totalSum == totalSum_check);
    std::cout << "Passed, totalSum_check = " << totalSum_check << std::endl;

    return 0;
}

CMakeLists.txt cmake_minimum_required(VERSION 3.0.0) project(2023-10-31 VERSION 0.1.0)

set(CMAKE_CXX_STANDARD 17) set(CMAKE_CXX_STANDARD_REQUIRED ON)

find_package(OpenMP REQUIRED) if(OPENMP_FOUND) message("[COMPLETE]:OPENMP FOUND") message("OpenMP_C_LIBRARIES = ${OpenMP_C_LIBRARIES}") else(OPENMP_FOUND) message("[ERROR]:OPENMP NOT FOUND") endif(OPENMP_FOUND)

add_executable(Test test.cpp) target_compile_options(Test PRIVATE -O3) target_link_libraries(Test pthread OpenMP::OpenMP_CXX)

Information

  1. CPU Name: Intel(R) Xeon(R) Silver 4214 CPU @ 2.20GHz (cores: 12 , threads per core: 1)
  2. Operating system: Linux (Ubuntu 18.04.1 LTS)
  3. Name and version of C++ compiler: GCC 9.1.0 x86_64
  4. Full command used for compiling, including all compiler flags: (see CMakeLists.txt)
  5. Thread pool library version: "v3.5.0 (2023-05-25)"

Behavior

When I set the label to 0, it passed. However, when the label is set to 1, I encountered a compilation error:

error: conversion from ‘std::conditional_t<true, void, std::vector<void, std::allocator > >’ {aka ‘void’} to non-scalar type ‘const std::vector’ requested.

This error points to the line:

const std::vector<uint64_t> sum_vec = myPool.parallelize_loop(num, addF).get();
bshoshany commented 1 year ago

Hi @PengBo410 and thanks for the bug report!

Before I can test it, I need two things:

  1. Please provide a minimal working example. This must be a complete program that I can compile as is, without adding anything.
  2. Please indicate the following information:
PengBo410 commented 1 year ago

I provided detailed information

guilhermelawless commented 1 year ago

Looks like a compiler bug that was fixed, works from gcc 9.3. https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93442

https://godbolt.org/z/cvThqrb5E

bshoshany commented 1 year ago

Thanks @guilhermelawless! Since this is a compiler bug, and not a bug in the thread pool library itself, I am closing this issue.