Skip to content

Commit

Permalink
Fix C++20 compatibility issues and faster integer hashing (#55)
Browse files Browse the repository at this point in the history
  • Loading branch information
DanielLiamAnderson authored Apr 24, 2023
1 parent 5af0694 commit 2e55a03
Show file tree
Hide file tree
Showing 6 changed files with 114 additions and 36 deletions.
77 changes: 65 additions & 12 deletions .github/workflows/build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ env:

jobs:
build:
name: ${{ matrix.config.os }} ${{ matrix.config.cxx }} ${{ matrix.config.note }} (${{ matrix.build_type }})
name: ${{ matrix.config.os }} ${{ matrix.config.cxx }} (C++17) ${{ matrix.config.note }} (${{ matrix.build_type }})
if: github.event_name != 'pull_request' || github.event.pull_request.head.repo.full_name != github.event.pull_request.base.repo.full_name
runs-on: ${{ matrix.config.os }}
defaults:
Expand Down Expand Up @@ -97,7 +97,7 @@ jobs:
ctest -C ${{ matrix.build_type }} --no-tests=error --output-on-failure
msvc:
name: windows-latest MSVC 19 (${{matrix.build_type}})
name: windows-latest MSVC 19 (C++17) (${{matrix.build_type}})
if: github.event_name != 'pull_request' || github.event.pull_request.head.repo.full_name != github.event.pull_request.base.repo.full_name
runs-on: windows-latest
strategy:
Expand Down Expand Up @@ -248,35 +248,88 @@ jobs:
# cd build
# ctest -C Debug --no-tests=error --output-on-failure

cpp20:
name: Ubuntu-22.04 ${{ matrix.config.cxx }} (C++20) (Debug)
if: github.event_name != 'pull_request' || github.event.pull_request.head.repo.full_name != github.event.pull_request.base.repo.full_name
runs-on: ubuntu-22.04
defaults:
run:
shell: bash
strategy:
matrix:
config:
- { cc: "gcc-12", cxx: "g++-12" }
- { cc: "clang-15", cxx: "clang++-15" }

steps:
- uses: actions/checkout@v2

- name: Set up toolchain repositories
run: |
sudo apt-get update
sudo add-apt-repository ppa:ubuntu-toolchain-r/test
wget -O - https://apt.llvm.org/llvm-snapshot.gpg.key | sudo apt-key add -
sudo apt-add-repository "deb http://apt.llvm.org/jammy/ llvm-toolchain-jammy-15 main"
- name: Install compiler
run: |
sudo apt-get -qq install cmake make ${{matrix.config.cc}} ${{matrix.config.cxx}}
- name: Configure
run: |
mkdir build && cd build
CC=${{matrix.config.cc}} CXX=${{matrix.config.cxx}} cmake -DCMAKE_BUILD_TYPE=Debug -DPARLAY_TEST=On -DPARLAY_BENCHMARK=On -DPARLAY_EXAMPLES=On -DPARLAY_USE_CXX_20=On ..
- name: Build
run: |
cd build
cmake --build . --config Debug
- name: Test
run: |
cd build
ctest -C Debug --no-tests=error --output-on-failure
# cilk-plus:
# name: ubuntu-18.04 Cilk Plus GCC-7 (Debug)
# name: windows-2019 WSL Ubuntu-18.04 Cilk Plus GCC-7 (Debug)
# if: github.event_name != 'pull_request' || github.event.pull_request.head.repo.full_name != github.event.pull_request.base.repo.full_name
# runs-on: ubuntu-18.04 # Note: GCC-7 on Ubuntu-20 removed Cilk Plus, so we have to go back in time
#
# # Note: GCC-7 on Ubuntu 20 removed Cilk Plus, so we have to go back in time to
# # Ubuntu 18 which GitHub stopped supporting, so we go through WSL instead.
# runs-on: windows-2019
# defaults:
# run:
# shell: wsl-bash {0}
# steps:
# - uses: actions/checkout@v2
# - uses: Vampire/setup-wsl@v1
# with:
# distribution: Ubuntu-18.04
#
# - name: Install GCC-7
# shell: bash
# run: |
# sudo apt-get -qq install gcc-7 g++-7
# sudo apt-get update
# sudo apt-get -qq install gcc-7 g++-7 cmake make
#
# - name: Install CMake
# run: |
# wget --quiet https://github.com/Kitware/CMake/releases/download/v3.18.2/cmake-3.18.2-Linux-x86_64.tar.gz
# tar -xf cmake-3.18.2-Linux-x86_64.tar.gz
#
# - name: Configure
# shell: bash
# run: |
# mkdir build && cd build
# CC=gcc-7 CXX=g++-7 cmake -DCMAKE_BUILD_TYPE=Debug -DPARLAY_TEST=On -DPARLAY_CILKPLUS=On -DPARLAY_BENCHMARK=On -DPARLAY_EXAMPLES=On ..
# CC=gcc-7 CXX=g++-7 ../cmake-3.18.2-Linux-x86_64/bin/cmake -DCMAKE_BUILD_TYPE=Debug -DPARLAY_TEST=On -DPARLAY_CILKPLUS=On -DPARLAY_BENCHMARK=On -DPARLAY_EXAMPLES=On ..
#
# - name: Build
# shell: bash
# run: |
# cd build
# cmake --build . --config Debug
# ../cmake-3.18.2-Linux-x86_64/bin/cmake --build . --config Debug
#
# - name: Test
# shell: bash
# run: |
# cd build
# ctest -C Debug --no-tests=error --output-on-failure
# ../cmake-3.18.2-Linux-x86_64/bin/ctest -C Debug --no-tests=error --output-on-failure

opencilk:
name: ubuntu-22.04 OpenCilk 2.0.0 (Clang 15) (Debug)
Expand Down
10 changes: 8 additions & 2 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
# -------------------------------------------------------------------

cmake_minimum_required(VERSION 3.14)
project(PARLAY VERSION 2.1.8
project(PARLAY VERSION 2.1.9
DESCRIPTION "A collection of parallel algorithms and other support for parallelism in C++"
LANGUAGES CXX)

Expand Down Expand Up @@ -50,7 +50,13 @@ set(PARLAY_INCLUDE_DIR "${PARLAY_SOURCE_DIR}/include")
target_include_directories(parlay INTERFACE
$<BUILD_INTERFACE:${PARLAY_INCLUDE_DIR}>
$<INSTALL_INTERFACE:${CMAKE_INSTALL_INCLUDEDIR}>)
target_compile_features(parlay INTERFACE cxx_std_17)

option(PARLAY_USE_CXX_20 "Require Parlay to build with C++20")
if (PARLAY_USE_CXX_20)
target_compile_features(parlay INTERFACE cxx_std_20)
else()
target_compile_features(parlay INTERFACE cxx_std_17)
endif()

# Link against system threads
find_package(Threads REQUIRED)
Expand Down
53 changes: 34 additions & 19 deletions include/parlay/internal/atomic_wait.h
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,10 @@ The strategy is chosen this way, by platform:
//#define __NO_SPIN
//#define __NO_WAIT

#ifndef __ATOMIC_WAIT_INCLUDED
#define __ATOMIC_WAIT_INCLUDED
#ifndef PARLAY_INTERNAL_ATOMIC_WAIT_H_
#define PARLAY_INTERNAL_ATOMIC_WAIT_H_

#include "../type_traits.h"

#ifndef __cpp_lib_atomic_wait

Expand Down Expand Up @@ -355,21 +357,26 @@ __ABI void __cxx_atomic_wait(_Tp const* ptr, _Tp const val, int order) {

namespace parlay {

template <class _Tp, class _Tv>
__ABI void atomic_wait_explicit(std::atomic<_Tp> const* a, _Tv val, std::memory_order order) {
__cxx_atomic_wait((const _Tp*)a, (_Tp)val, (int)order); // cppcheck-suppress cstyleCast
// Note: We use parlay::type_identity_t here to force a non-deduced context for _Tp. We
// can't use typename std::atomic<_Tp>::value_type as the C++20 standard does, because
// value_type is not a member of std::atomic in older compiler version (it was added in
// a defect report, P0558R1, but doesn't appear to have been retroactively fixed).

template <class _Tp>
__ABI void atomic_wait_explicit(const std::atomic<_Tp>* a, parlay::type_identity_t<_Tp> val, std::memory_order order) {
__cxx_atomic_wait((const _Tp*)a, val, (int)order); // cppcheck-suppress cstyleCast
}
template <class _Tp, class _Tv>
__ABI void atomic_wait(std::atomic<_Tp> const* a, _Tv val) {
parlay::atomic_wait_explicit(a, val, std::memory_order_seq_cst);
template <class _Tp>
__ABI void atomic_wait(const std::atomic<_Tp>* a, parlay::type_identity_t<_Tp> val) {
__cxx_atomic_wait((const _Tp*)a, val, (int)std::memory_order_seq_cst); // cppcheck-suppress cstyleCast
}
template <class _Tp>
__ABI void atomic_notify_one(std::atomic<_Tp> const* a) {
__cxx_atomic_notify_one((const _Tp*)a); // cppcheck-suppress cstyleCast
__ABI void atomic_notify_one(std::atomic<_Tp>* a) {
__cxx_atomic_notify_one((const _Tp*)a); // cppcheck-suppress cstyleCast
}
template <class _Tp>
__ABI void atomic_notify_all(std::atomic<_Tp> const* a) {
__cxx_atomic_notify_all((const _Tp*)a); // cppcheck-suppress cstyleCast
__ABI void atomic_notify_all(std::atomic<_Tp>* a) {
__cxx_atomic_notify_all((const _Tp*)a); // cppcheck-suppress cstyleCast
}

}
Expand All @@ -391,20 +398,28 @@ extern inline contended_t * __contention(volatile void const * p) {

namespace parlay {

template <class _Tp, class _Tv>
const auto atomic_wait_explicit = std::atomic_wait_explicit<_Tp, _Tv>;
template <class _Tp>
void atomic_wait_explicit(const std::atomic<_Tp>* a, parlay::type_identity_t<_Tp> val, std::memory_order order) {
std::atomic_wait_explicit(a, val, order);
}

template <class _Tp, class _Tv>
const auto atomic_wait = std::atomic_wait<_Tp, _Tv>;
template <class _Tp>
void atomic_wait(const std::atomic<_Tp>* a, parlay::type_identity_t<_Tp> val) {
std::atomic_wait(a, val);
}

template <class _Tp>
const auto atomic_notify_one = std::atomic_notify_one<_Tp>;
void atomic_notify_one(std::atomic<_Tp>* a) {
std::atomic_notify_one(a);
}

template <class _Tp>
const auto atomic_notify_all = std::atomic_notify_all<_Tp>;
void atomic_notify_all(std::atomic<_Tp>* a) {
std::atomic_notify_all(a);
}

}

#endif // !__cpp_lib_atomic_wait

#endif //__ATOMIC_WAIT_INCLUDED
#endif //PARLAY_INTERNAL_ATOMIC_WAIT_H_
2 changes: 1 addition & 1 deletion include/parlay/internal/sequence_base.h
Original file line number Diff line number Diff line change
Expand Up @@ -387,7 +387,7 @@ struct alignas(uint64_t) sequence_base {

value_type* data() { return elements.data(); }
const value_type* data() const { return elements.data(); }
};
} PARLAY_PACKED;


// Store either a short or a long sequence. By default, we
Expand Down
5 changes: 3 additions & 2 deletions include/parlay/utilities.h
Original file line number Diff line number Diff line change
Expand Up @@ -161,9 +161,10 @@ template<typename T, typename Enable = void>
struct hash : public std::hash<T> { };

template <typename T>
struct hash<T,typename std::enable_if_t<std::is_integral_v<T>>> {
struct hash<T, typename std::enable_if_t<std::is_integral_v<T>>> {
size_t operator()(const T& p) const {
return hash64_2(p);}
return p * UINT64_C(0xbf58476d1ce4e5b9);
}
};

// Hashing for std::pairs
Expand Down
3 changes: 3 additions & 0 deletions test/test_parallel.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ TEST(TestParallel, TestParDoWorkerIds) {

TEST(TestParallel, TestParDoUncopyableF) {
struct F {
F() = default;
F(const F&) = delete;
F(F&&) = default;
void operator()() const { }
Expand Down Expand Up @@ -111,6 +112,7 @@ TEST(TestParallel, TestParForUncopyableF) {
size_t n = 100000;
std::vector<int> v(n);
struct F {
F(std::vector<int>& v_) : v(v_) {}
F(const F&) = delete;
std::vector<int>& v;
void operator()(size_t i) const { v[i] = i; } };
Expand All @@ -125,6 +127,7 @@ TEST(TestParallel, TestParForUncopyableTempF) {
size_t n = 100000;
std::vector<int> v(n);
struct F {
F(std::vector<int>& v_) : v(v_) {}
F(const F&) = delete;
std::vector<int>& v;
void operator()(size_t i) const { v[i] = i; } };
Expand Down

0 comments on commit 2e55a03

Please sign in to comment.