Confine the atomic reinterpret_cast to one shim, use std::atomic_ref at C++20 (#1791)
* Replace the atomic reinterpret_cast with an AtomicRef shim * Scope the C++20 deprecation suppression to the shared_ptr atomics * Move the shared_ptr atomic helpers into atomic_compat.h * Apply review: 32-bit MSVC fallback, comment accuracy, include hygiene * Hoist the MSVC includes back to file scope * Address review: enumerate CI jobs, reject 32-bit MSVC * Name the MSVC atomic backend condition once * Correct atomic_compat's comment on what survives at C++20 * Back off to a thin C++17 cast shim; std::atomic_ref at C++20 * Drop the AtomicRef concurrency tests; they cannot detect breakage
This commit is contained in:
@@ -91,17 +91,42 @@ jobs:
|
||||
name: meshes-windows
|
||||
path: meshes-windows/
|
||||
build:
|
||||
name: GCC ${{matrix.gcc}} (TBB:${{matrix.parallelization}})
|
||||
name: ${{matrix.name}}
|
||||
timeout-minutes: 45
|
||||
strategy:
|
||||
matrix:
|
||||
parallelization: [OFF, ON]
|
||||
os: [ubuntu-24.04]
|
||||
gcc: [13, 14]
|
||||
include:
|
||||
- parallelization: ON
|
||||
os: ubuntu-22.04
|
||||
- name: GCC 11 (TBB:ON)
|
||||
gcc: 11
|
||||
parallelization: ON
|
||||
os: ubuntu-22.04
|
||||
std: 17
|
||||
- name: GCC 13 (TBB:OFF)
|
||||
gcc: 13
|
||||
parallelization: OFF
|
||||
os: ubuntu-24.04
|
||||
std: 17
|
||||
- name: GCC 13 (TBB:ON)
|
||||
gcc: 13
|
||||
parallelization: ON
|
||||
os: ubuntu-24.04
|
||||
std: 17
|
||||
- name: GCC 14 (TBB:OFF)
|
||||
gcc: 14
|
||||
parallelization: OFF
|
||||
os: ubuntu-24.04
|
||||
std: 17
|
||||
- name: GCC 14 (TBB:ON)
|
||||
gcc: 14
|
||||
parallelization: ON
|
||||
os: ubuntu-24.04
|
||||
std: 17
|
||||
# One strict lane at C++20, to catch what it deprecates or redefines.
|
||||
- name: GCC 14 (TBB:ON, C++20)
|
||||
gcc: 14
|
||||
parallelization: ON
|
||||
os: ubuntu-24.04
|
||||
std: 20
|
||||
runs-on: ${{ matrix.os }}
|
||||
env:
|
||||
CC: gcc-${{ matrix.gcc }}
|
||||
@@ -124,6 +149,7 @@ jobs:
|
||||
run: |
|
||||
cmake \
|
||||
-DCMAKE_BUILD_TYPE=RelWithDebInfo \
|
||||
-DCMAKE_CXX_STANDARD=${{matrix.std}} \
|
||||
-DBUILD_SHARED_LIBS=ON \
|
||||
-DMANIFOLD_STRICT=ON \
|
||||
-DMANIFOLD_PYBIND=ON \
|
||||
|
||||
+4
-2
@@ -15,9 +15,11 @@
|
||||
cmake_minimum_required(VERSION 3.18)
|
||||
project(manifold LANGUAGES CXX)
|
||||
|
||||
# Use C++17
|
||||
# C++17 by default, guarded so -DCMAKE_CXX_STANDARD and parent projects work.
|
||||
set(CMAKE_CXX_EXTENSIONS OFF)
|
||||
set(CMAKE_CXX_STANDARD 17)
|
||||
if(NOT DEFINED CMAKE_CXX_STANDARD OR CMAKE_CXX_STANDARD LESS 17)
|
||||
set(CMAKE_CXX_STANDARD 17)
|
||||
endif()
|
||||
set(CMAKE_CXX_STANDARD_REQUIRED ON)
|
||||
|
||||
set(CMAKE_VERBOSE_MAKEFILE ON)
|
||||
|
||||
@@ -345,16 +345,16 @@ class Manifold {
|
||||
// Propagates through copy ctor / op= (raw copy preserves the attachment).
|
||||
// Manifold-returning ops do *not* propagate it: derived Manifolds get a
|
||||
// null ctx_. Eager ops (Status, Refine family) snapshot ctx_ to observe
|
||||
// their in-call work; the snapshot uses std::atomic_load, which pins the
|
||||
// their in-call work; the snapshot uses an atomic load, which pins the
|
||||
// Impl across long-running evaluations even if a concurrent op= reseats
|
||||
// ctx_ mid-eval.
|
||||
//
|
||||
// Accessed only via std::atomic_load / std::atomic_store: no const method
|
||||
// mutates ctx_, but op= and the copy ctor write it on a Manifold that
|
||||
// Accessed only atomically (AtomicLoadShared/AtomicStoreShared): no const
|
||||
// method mutates ctx_, but op= and the copy ctor write it on a Manifold that
|
||||
// may be concurrently observed by const methods on other threads. The
|
||||
// atomic-shared-ptr free functions give a torn-read-free snapshot
|
||||
// without taking a lock. (pNode_ uses a mutex instead because lazy CSG
|
||||
// eval mutates it through const methods, which atomic_load can't model.)
|
||||
// eval mutates it through const methods, which an atomic load can't model.)
|
||||
std::shared_ptr<ExecutionContext::Impl> ctx_;
|
||||
|
||||
std::shared_ptr<CsgNode> LoadPNode() const;
|
||||
|
||||
@@ -42,6 +42,7 @@ set(
|
||||
|
||||
set(
|
||||
MANIFOLD_PRIVATE_HDRS
|
||||
atomic_compat.h
|
||||
boolean2.h
|
||||
boolean2_diagnostics.h
|
||||
boolean3.h
|
||||
|
||||
@@ -0,0 +1,177 @@
|
||||
// Copyright 2026 The Manifold Authors.
|
||||
//
|
||||
// Licensed under the Apache License, Version 2.0 (the "License");
|
||||
// you may not use this file except in compliance with the License.
|
||||
// You may obtain a copy of the License at
|
||||
//
|
||||
// http://www.apache.org/licenses/LICENSE-2.0
|
||||
//
|
||||
// Unless required by applicable law or agreed to in writing, software
|
||||
// distributed under the License is distributed on an "AS IS" BASIS,
|
||||
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
|
||||
// See the License for the specific language governing permissions and
|
||||
// limitations under the License.
|
||||
//
|
||||
// C++17 stand-ins for atomic facilities the library would otherwise get from
|
||||
// C++20. The two halves have different lifetimes:
|
||||
//
|
||||
// - `AtomicRef<T>` is `std::atomic_ref` (P0019R8) at C++20, and at C++17 a
|
||||
// thin wrapper over the reinterpret_cast it replaces. The cast is still
|
||||
// undefined behavior, but it is now in one place instead of scattered
|
||||
// across the call sites, its preconditions are static_asserts rather than
|
||||
// assumptions, and the C++20 lane builds the defined version.
|
||||
// - `AtomicLoadShared` / `AtomicStoreShared` wrap the `std::atomic_load` and
|
||||
// `std::atomic_store` shared_ptr overloads, which C++20 deprecates and
|
||||
// C++26 removes (P2869). These do not go away at C++20: the replacement
|
||||
// changes the member's type to `std::atomic<std::shared_ptr<T>>` rather
|
||||
// than how it is accessed, and libc++ still leaves
|
||||
// `__cpp_lib_atomic_shared_ptr` undefined. Revisit when the baseline moves
|
||||
// and libc++ ships it.
|
||||
|
||||
#pragma once
|
||||
#include <atomic>
|
||||
#include <cstdint>
|
||||
#include <memory>
|
||||
#include <type_traits>
|
||||
|
||||
#include "manifold/optional_assert.h"
|
||||
|
||||
namespace manifold {
|
||||
|
||||
#if defined(__cpp_lib_atomic_ref) // C++20: the real thing
|
||||
|
||||
template <typename T>
|
||||
using AtomicRef = std::atomic_ref<T>;
|
||||
|
||||
#else // C++17: the cast, confined
|
||||
|
||||
/**
|
||||
* A C++17 stand-in for std::atomic_ref. Reinterprets the referenced object as
|
||||
* `std::atomic<T>` and forwards to it, which is what the call sites did before
|
||||
* this type existed.
|
||||
*
|
||||
* That cast is undefined behavior: the object is not an atomic object, and
|
||||
* nothing in the standard says the two are layout-compatible. It works on
|
||||
* every ABI we ship, and the static_asserts below check the parts a compiler
|
||||
* can check, but they cannot make it defined. The point of the type is that
|
||||
* there is now exactly one cast to audit, and that at C++20 it is replaced
|
||||
* wholesale by the alias above rather than being repaired.
|
||||
*
|
||||
* The referenced object must outlive the AtomicRef, and while any AtomicRef
|
||||
* refers to it every access must go through one.
|
||||
*
|
||||
* Deliberately a subset of std::atomic_ref: only what the call sites use, so
|
||||
* the C++20 alias is a superset and anything written against this compiles
|
||||
* either way. Missing on purpose are `operator=`, `operator T`, the other
|
||||
* fetch_* operations, the compound assignments, and wait/notify. Adding one
|
||||
* here is fine; relying on one that only the alias has builds on the C++20
|
||||
* lane and fails everywhere else.
|
||||
*/
|
||||
template <typename T>
|
||||
class AtomicRef {
|
||||
// Every use here is a scalar. Narrower than std::atomic_ref's trivially-
|
||||
// copyable contract, and it keeps padded types - whose padding bytes would
|
||||
// have to be cleared before each compare_exchange - out of the cast.
|
||||
static_assert(std::is_scalar<T>::value, "AtomicRef requires a scalar type");
|
||||
// What makes the cast work in practice. A lock-based std::atomic<T> carries
|
||||
// a lock alongside the value, so these would catch it.
|
||||
static_assert(sizeof(std::atomic<T>) == sizeof(T),
|
||||
"std::atomic<T> must be the same size as T");
|
||||
static_assert(alignof(std::atomic<T>) == alignof(T),
|
||||
"std::atomic<T> must be aligned like T");
|
||||
static_assert(std::atomic<T>::is_always_lock_free,
|
||||
"AtomicRef requires a lock-free std::atomic<T>");
|
||||
|
||||
public:
|
||||
explicit AtomicRef(T& obj) : ref_(reinterpret_cast<std::atomic<T>&>(obj)) {
|
||||
// Natural alignment is not implied by alignof(T): 32-bit x86 gives
|
||||
// alignof(double) == 4. Compiles away without MANIFOLD_DEBUG. The C++20
|
||||
// alias has no equivalent check - std::atomic_ref states the requirement
|
||||
// as required_alignment and leaves violations undefined - so this catches
|
||||
// a misaligned caller only on the C++17 side.
|
||||
DEBUG_ASSERT(reinterpret_cast<uintptr_t>(&obj) % sizeof(T) == 0, logicErr,
|
||||
"AtomicRef requires an object aligned to its own size");
|
||||
}
|
||||
|
||||
T load(std::memory_order order = std::memory_order_seq_cst) const {
|
||||
return ref_.load(order);
|
||||
}
|
||||
|
||||
void store(T desired,
|
||||
std::memory_order order = std::memory_order_seq_cst) const {
|
||||
ref_.store(desired, order);
|
||||
}
|
||||
|
||||
T exchange(T desired,
|
||||
std::memory_order order = std::memory_order_seq_cst) const {
|
||||
return ref_.exchange(desired, order);
|
||||
}
|
||||
|
||||
// As in std::atomic_ref, `expected` is updated with the current value when
|
||||
// the exchange fails. Only the weak form may fail spuriously, so a caller
|
||||
// that treats failure as evidence about the stored value needs the strong
|
||||
// one.
|
||||
bool compare_exchange_weak(
|
||||
T& expected, T desired,
|
||||
std::memory_order order = std::memory_order_seq_cst) const {
|
||||
return ref_.compare_exchange_weak(expected, desired, order);
|
||||
}
|
||||
|
||||
bool compare_exchange_weak(T& expected, T desired, std::memory_order success,
|
||||
std::memory_order failure) const {
|
||||
return ref_.compare_exchange_weak(expected, desired, success, failure);
|
||||
}
|
||||
|
||||
bool compare_exchange_strong(
|
||||
T& expected, T desired,
|
||||
std::memory_order order = std::memory_order_seq_cst) const {
|
||||
return ref_.compare_exchange_strong(expected, desired, order);
|
||||
}
|
||||
|
||||
// Integral only: C++17's std::atomic has no floating-point fetch_add, so
|
||||
// this gate is what keeps `double` on AtomicAdd's compare_exchange_weak
|
||||
// loop. Keep it.
|
||||
template <typename U = T>
|
||||
std::enable_if_t<std::is_integral<U>::value, T> fetch_add(
|
||||
T arg, std::memory_order order = std::memory_order_seq_cst) const {
|
||||
return ref_.fetch_add(arg, order);
|
||||
}
|
||||
|
||||
private:
|
||||
std::atomic<T>& ref_;
|
||||
};
|
||||
|
||||
#endif // std::atomic_ref vs the C++17 cast
|
||||
|
||||
// Suppresses every deprecation inside its region, so keep the regions to a
|
||||
// single expression. Neither GCC nor Clang offers per-symbol suppression.
|
||||
#if defined(_MSC_VER) && !defined(__clang__)
|
||||
#define MANIFOLD_IGNORE_DEPRECATED_BEGIN \
|
||||
__pragma(warning(push)) __pragma(warning(disable : 4996))
|
||||
#define MANIFOLD_IGNORE_DEPRECATED_END __pragma(warning(pop))
|
||||
#else
|
||||
#define MANIFOLD_IGNORE_DEPRECATED_BEGIN \
|
||||
_Pragma("GCC diagnostic push") \
|
||||
_Pragma("GCC diagnostic ignored \"-Wdeprecated-declarations\"")
|
||||
#define MANIFOLD_IGNORE_DEPRECATED_END _Pragma("GCC diagnostic pop")
|
||||
#endif
|
||||
|
||||
// Atomic access to a plain `shared_ptr`, which C++17 can only express through
|
||||
// these overloads. There is no atomic_ref equivalent: atomic_ref requires a
|
||||
// trivially copyable type, so the C++20 replacement changes the member's type
|
||||
// rather than how it is accessed.
|
||||
template <typename T>
|
||||
std::shared_ptr<T> AtomicLoadShared(const std::shared_ptr<T>* ptr) {
|
||||
MANIFOLD_IGNORE_DEPRECATED_BEGIN
|
||||
return std::atomic_load(ptr);
|
||||
MANIFOLD_IGNORE_DEPRECATED_END
|
||||
}
|
||||
|
||||
template <typename T>
|
||||
void AtomicStoreShared(std::shared_ptr<T>* ptr, std::shared_ptr<T> value) {
|
||||
MANIFOLD_IGNORE_DEPRECATED_BEGIN
|
||||
std::atomic_store(ptr, std::move(value));
|
||||
MANIFOLD_IGNORE_DEPRECATED_END
|
||||
}
|
||||
|
||||
} // namespace manifold
|
||||
+10
-11
@@ -20,6 +20,7 @@
|
||||
#include <intrin.h>
|
||||
#endif
|
||||
|
||||
#include "atomic_compat.h"
|
||||
#include "utils.h"
|
||||
#include "vec.h"
|
||||
|
||||
@@ -62,23 +63,21 @@ inline uint32_t CeilLog2(size_t value) {
|
||||
|
||||
template <typename T>
|
||||
T AtomicCAS(T& target, T compare, T val) {
|
||||
std::atomic<T>& tar = reinterpret_cast<std::atomic<T>&>(target);
|
||||
// Must be the strong form: Insert treats a returned `kOpen` as proof it
|
||||
// claimed the slot, and a spurious weak failure would report exactly that
|
||||
// without having written the key.
|
||||
manifold::AtomicRef<T> tar(target);
|
||||
tar.compare_exchange_strong(compare, val, std::memory_order_acq_rel);
|
||||
return compare;
|
||||
}
|
||||
|
||||
template <typename T>
|
||||
void AtomicStore(T& target, T val) {
|
||||
std::atomic<T>& tar = reinterpret_cast<std::atomic<T>&>(target);
|
||||
// release is good enough, although not really something general
|
||||
tar.store(val, std::memory_order_release);
|
||||
}
|
||||
|
||||
template <typename T>
|
||||
T AtomicLoad(const T& target) {
|
||||
const std::atomic<T>& tar = reinterpret_cast<const std::atomic<T>&>(target);
|
||||
// acquire is good enough, although not general
|
||||
return tar.load(std::memory_order_acquire);
|
||||
// acquire is good enough, although not general. The const_cast is the
|
||||
// caller's business rather than AtomicRef's, so the C++20 path can be a
|
||||
// plain alias for std::atomic_ref, which takes only a mutable reference.
|
||||
return manifold::AtomicRef<T>(const_cast<T&>(target))
|
||||
.load(std::memory_order_acquire);
|
||||
}
|
||||
|
||||
} // namespace
|
||||
|
||||
+1
-2
@@ -148,8 +148,7 @@ void Manifold::Impl::RemoveUnreferencedVerts() {
|
||||
for_each_n(policy, countAt(0), halfedge_.size(), [&keep, this](int edge) {
|
||||
const int startVert = halfedge_.Start(edge);
|
||||
if (startVert >= 0) {
|
||||
reinterpret_cast<std::atomic<int>*>(&keep[startVert])
|
||||
->store(1, std::memory_order_relaxed);
|
||||
AtomicRef<int>(keep[startVert]).store(1, std::memory_order_relaxed);
|
||||
}
|
||||
});
|
||||
|
||||
|
||||
+11
-10
@@ -14,6 +14,7 @@
|
||||
|
||||
#include <algorithm>
|
||||
|
||||
#include "atomic_compat.h"
|
||||
#include "boolean3.h"
|
||||
#include "csg_tree.h"
|
||||
#include "execution_impl.h"
|
||||
@@ -128,7 +129,7 @@ Manifold& Manifold::operator=(Manifold&&) noexcept = default;
|
||||
Manifold::Manifold(const Manifold& other) {
|
||||
std::lock_guard<std::mutex> lock(*other.pNodeMutex_);
|
||||
pNode_ = other.pNode_;
|
||||
std::atomic_store(&ctx_, std::atomic_load(&other.ctx_));
|
||||
AtomicStoreShared(&ctx_, AtomicLoadShared(&other.ctx_));
|
||||
}
|
||||
|
||||
Manifold::Manifold(std::shared_ptr<CsgNode> pNode) : pNode_(pNode) {}
|
||||
@@ -156,7 +157,7 @@ Manifold& Manifold::operator=(const Manifold& other) {
|
||||
if (this != &other) {
|
||||
std::scoped_lock lock(*pNodeMutex_, *other.pNodeMutex_);
|
||||
pNode_ = other.pNode_;
|
||||
std::atomic_store(&ctx_, std::atomic_load(&other.ctx_));
|
||||
AtomicStoreShared(&ctx_, AtomicLoadShared(&other.ctx_));
|
||||
}
|
||||
return *this;
|
||||
}
|
||||
@@ -170,7 +171,7 @@ Manifold& Manifold::operator=(const Manifold& other) {
|
||||
*/
|
||||
Manifold Manifold::WithContext(const ExecutionContext& ctx) const {
|
||||
Manifold result = *this;
|
||||
std::atomic_store(&result.ctx_, ctx.impl_);
|
||||
AtomicStoreShared(&result.ctx_, ctx.impl_);
|
||||
return result;
|
||||
}
|
||||
|
||||
@@ -305,7 +306,7 @@ Manifold::Error Manifold::Status() const {
|
||||
// expression -- through the lazy eval inside GetCsgLeafNode -- so a
|
||||
// concurrent op= reseating ctx_ on this Manifold can't free the Impl out
|
||||
// from under us.
|
||||
return GetCsgLeafNode(std::atomic_load(&ctx_).get()).GetImpl()->status_;
|
||||
return GetCsgLeafNode(AtomicLoadShared(&ctx_).get()).GetImpl()->status_;
|
||||
}
|
||||
/**
|
||||
* The number of vertices in the Manifold.
|
||||
@@ -788,7 +789,7 @@ Manifold Manifold::SmoothOut(double minSharpAngle, double minSmoothness) const {
|
||||
* @param n The number of pieces to split every edge into. Must be > 1.
|
||||
*/
|
||||
Manifold Manifold::Refine(int n) const {
|
||||
auto ctx = std::atomic_load(&ctx_);
|
||||
auto ctx = AtomicLoadShared(&ctx_);
|
||||
auto leafImpl = GetCsgLeafNode(ctx.get()).GetImpl();
|
||||
if (leafImpl->status_ != Error::NoError)
|
||||
return PropagateStatus(leafImpl->status_);
|
||||
@@ -813,7 +814,7 @@ Manifold Manifold::Refine(int n) const {
|
||||
*/
|
||||
Manifold Manifold::RefineToLength(double length) const {
|
||||
length = std::abs(length);
|
||||
auto ctx = std::atomic_load(&ctx_);
|
||||
auto ctx = AtomicLoadShared(&ctx_);
|
||||
auto leafImpl = GetCsgLeafNode(ctx.get()).GetImpl();
|
||||
if (leafImpl->status_ != Error::NoError)
|
||||
return PropagateStatus(leafImpl->status_);
|
||||
@@ -842,7 +843,7 @@ Manifold Manifold::RefineToLength(double length) const {
|
||||
*/
|
||||
Manifold Manifold::RefineToTolerance(double tolerance) const {
|
||||
tolerance = std::abs(tolerance);
|
||||
auto ctx = std::atomic_load(&ctx_);
|
||||
auto ctx = AtomicLoadShared(&ctx_);
|
||||
auto leafImpl = GetCsgLeafNode(ctx.get()).GetImpl();
|
||||
if (leafImpl->status_ != Error::NoError)
|
||||
return PropagateStatus(leafImpl->status_);
|
||||
@@ -1011,7 +1012,7 @@ Manifold Manifold::TrimByPlane(vec3 normal, double originOffset) const {
|
||||
* @param other The other manifold to minkowski sum to this one.
|
||||
*/
|
||||
Manifold Manifold::MinkowskiSum(const Manifold& other) const {
|
||||
auto ctx = std::atomic_load(&ctx_);
|
||||
auto ctx = AtomicLoadShared(&ctx_);
|
||||
auto aImpl = GetCsgLeafNode(ctx.get()).GetImpl();
|
||||
if (aImpl->status_ != Error::NoError) return PropagateStatus(aImpl->status_);
|
||||
auto bImpl = other.GetCsgLeafNode(ctx.get()).GetImpl();
|
||||
@@ -1029,7 +1030,7 @@ Manifold Manifold::MinkowskiSum(const Manifold& other) const {
|
||||
* @param other The other manifold to minkowski subtract from this one.
|
||||
*/
|
||||
Manifold Manifold::MinkowskiDifference(const Manifold& other) const {
|
||||
auto ctx = std::atomic_load(&ctx_);
|
||||
auto ctx = AtomicLoadShared(&ctx_);
|
||||
auto aImpl = GetCsgLeafNode(ctx.get()).GetImpl();
|
||||
if (aImpl->status_ != Error::NoError) return PropagateStatus(aImpl->status_);
|
||||
auto bImpl = other.GetCsgLeafNode(ctx.get()).GetImpl();
|
||||
@@ -1076,7 +1077,7 @@ Manifold Manifold::Hull(const std::vector<vec3>& pts) {
|
||||
* Compute the convex hull of this manifold.
|
||||
*/
|
||||
Manifold Manifold::Hull() const {
|
||||
auto ctx = std::atomic_load(&ctx_);
|
||||
auto ctx = AtomicLoadShared(&ctx_);
|
||||
auto srcImpl = GetCsgLeafNode(ctx.get()).GetImpl();
|
||||
if (srcImpl->status_ != Error::NoError)
|
||||
return PropagateStatus(srcImpl->status_);
|
||||
|
||||
+2
-3
@@ -359,9 +359,8 @@ void Manifold::Impl::CalculateCurvature(int gaussianIdx, int meanIdx) {
|
||||
const int vert = halfedge_.Start(edge);
|
||||
const int propVert = halfedge_.Prop(edge);
|
||||
|
||||
auto old = std::atomic_exchange(
|
||||
reinterpret_cast<std::atomic<uint8_t>*>(&counters[propVert]),
|
||||
static_cast<uint8_t>(1));
|
||||
auto old = AtomicRef<uint8_t>(counters[propVert])
|
||||
.exchange(static_cast<uint8_t>(1));
|
||||
if (old == 1) continue;
|
||||
|
||||
for (int p = 0; p < oldNumProp; ++p) {
|
||||
|
||||
+2
-3
@@ -412,9 +412,8 @@ Vec<int> Manifold::Impl::VertHalfedge() const {
|
||||
for_each_n(autoPolicy(halfedge_.size(), 1e5), countAt(0), halfedge_.size(),
|
||||
[&vertHalfedge, &counters, this](const int idx) {
|
||||
const int start = halfedge_.Start(idx);
|
||||
auto old = std::atomic_exchange(
|
||||
reinterpret_cast<std::atomic<uint8_t>*>(&counters[start]),
|
||||
static_cast<uint8_t>(1));
|
||||
auto old = AtomicRef<uint8_t>(counters[start])
|
||||
.exchange(static_cast<uint8_t>(1));
|
||||
if (old == 1) return;
|
||||
// arbitrary, last one wins.
|
||||
vertHalfedge[start] = idx;
|
||||
|
||||
+2
-2
@@ -390,8 +390,8 @@ void Manifold::Impl::CompactProps(ExecutionContext::Impl* ctx) {
|
||||
auto policy = autoPolicy(numVerts, 1e5);
|
||||
|
||||
for_each_n(policy, countAt(0), halfedge_.size(), ctx, [this, &keep](int idx) {
|
||||
reinterpret_cast<std::atomic<int>*>(&keep[halfedge_.Prop(idx)])
|
||||
->store(1, std::memory_order_relaxed);
|
||||
AtomicRef<int>(keep[halfedge_.Prop(idx)])
|
||||
.store(1, std::memory_order_relaxed);
|
||||
});
|
||||
if (IsCancelled(ctx)) return;
|
||||
Vec<int> propOld2New(numVerts + 1, 0);
|
||||
|
||||
+10
-6
@@ -18,6 +18,7 @@
|
||||
#include <memory>
|
||||
#include <mutex>
|
||||
|
||||
#include "atomic_compat.h"
|
||||
#include "manifold/common.h"
|
||||
#include "vec.h"
|
||||
|
||||
@@ -62,21 +63,24 @@ void Permute(std::vector<T>& inOut, const Vec<T1>& new2Old) {
|
||||
gather(new2Old.begin(), new2Old.end(), tmp.begin(), inOut.begin());
|
||||
}
|
||||
|
||||
// Floating-point accumulation, since fetch_add takes integers only. The
|
||||
// relaxed seed and relaxed failure order are what libstdc++'s __fetch_add_flt
|
||||
// and libc++'s floating-point fetch_add both use: only the successful exchange
|
||||
// needs to synchronize.
|
||||
template <typename T>
|
||||
T AtomicAdd(T& target, T add) {
|
||||
std::atomic<T>& tar = reinterpret_cast<std::atomic<T>&>(target);
|
||||
T old_val = tar.load();
|
||||
AtomicRef<T> tar(target);
|
||||
T old_val = tar.load(std::memory_order_relaxed);
|
||||
while (!tar.compare_exchange_weak(old_val, old_val + add,
|
||||
std::memory_order_seq_cst)) {
|
||||
std::memory_order_seq_cst,
|
||||
std::memory_order_relaxed)) {
|
||||
}
|
||||
return old_val;
|
||||
}
|
||||
|
||||
template <>
|
||||
inline int AtomicAdd(int& target, int add) {
|
||||
std::atomic<int>& tar = reinterpret_cast<std::atomic<int>&>(target);
|
||||
int old_val = tar.fetch_add(add, std::memory_order_seq_cst);
|
||||
return old_val;
|
||||
return AtomicRef<int>(target).fetch_add(add);
|
||||
}
|
||||
|
||||
template <typename T>
|
||||
|
||||
@@ -22,6 +22,7 @@ enable_testing()
|
||||
set(
|
||||
SOURCE_FILES
|
||||
test_main.cpp
|
||||
utils_test.cpp
|
||||
polygon_test.cpp
|
||||
properties_test.cpp
|
||||
manifold_test.cpp
|
||||
|
||||
@@ -0,0 +1,140 @@
|
||||
// Copyright 2026 The Manifold Authors.
|
||||
//
|
||||
// Licensed under the Apache License, Version 2.0 (the "License");
|
||||
// you may not use this file except in compliance with the License.
|
||||
// You may obtain a copy of the License at
|
||||
//
|
||||
// http://www.apache.org/licenses/LICENSE-2.0
|
||||
//
|
||||
// Unless required by applicable law or agreed to in writing, software
|
||||
// distributed under the License is distributed on an "AS IS" BASIS,
|
||||
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
|
||||
// See the License for the specific language governing permissions and
|
||||
// limitations under the License.
|
||||
|
||||
#include "../src/utils.h"
|
||||
|
||||
#include <atomic>
|
||||
#include <cstdint>
|
||||
#include <iostream>
|
||||
|
||||
#include "../src/atomic_compat.h"
|
||||
#include "gtest/gtest.h"
|
||||
|
||||
using namespace manifold;
|
||||
|
||||
// Which backend this build selected is not otherwise visible. Report it so a
|
||||
// CI log says whether the lane covered the std::atomic_ref path or a fallback.
|
||||
TEST(Utils, AtomicRefBackend) {
|
||||
#if defined(__cpp_lib_atomic_ref)
|
||||
const char* backend = "std::atomic_ref";
|
||||
#else
|
||||
const char* backend = "C++17 cast";
|
||||
#endif
|
||||
RecordProperty("backend", backend);
|
||||
std::cout << "AtomicRef backend: " << backend << std::endl;
|
||||
}
|
||||
|
||||
// A failed compare_exchange reports the current value through `expected`,
|
||||
// which is what the AtomicAdd retry loop relies on.
|
||||
TEST(Utils, AtomicRefCompareExchangeReportsCurrentValue) {
|
||||
int value = 7;
|
||||
int expected = 3;
|
||||
EXPECT_FALSE(AtomicRef<int>(value).compare_exchange_strong(expected, 9));
|
||||
EXPECT_EQ(expected, 7);
|
||||
EXPECT_EQ(value, 7);
|
||||
|
||||
EXPECT_TRUE(AtomicRef<int>(value).compare_exchange_strong(expected, 9));
|
||||
EXPECT_EQ(value, 9);
|
||||
}
|
||||
|
||||
// std::atomic's layout and lock-freedom are asserted per width, and the cast
|
||||
// relies on both, so exercise every width plus a non-integer type.
|
||||
template <typename T>
|
||||
void ExpectRoundTrip(T a, T b) {
|
||||
SCOPED_TRACE(testing::Message() << "sizeof(T) = " << sizeof(T));
|
||||
T value = a;
|
||||
AtomicRef<T> ref(value);
|
||||
EXPECT_EQ(ref.load(), a);
|
||||
|
||||
ref.store(b, std::memory_order_relaxed);
|
||||
EXPECT_EQ(ref.load(std::memory_order_relaxed), b);
|
||||
ref.store(a, std::memory_order_release);
|
||||
EXPECT_EQ(ref.load(std::memory_order_acquire), a);
|
||||
|
||||
EXPECT_EQ(ref.exchange(b), a);
|
||||
EXPECT_EQ(value, b);
|
||||
|
||||
T expected = a; // wrong on purpose: the exchange must fail and report b
|
||||
EXPECT_FALSE(ref.compare_exchange_strong(expected, a));
|
||||
EXPECT_EQ(expected, b);
|
||||
EXPECT_TRUE(ref.compare_exchange_strong(expected, a));
|
||||
EXPECT_EQ(value, a);
|
||||
}
|
||||
|
||||
TEST(Utils, AtomicRefRoundTripsEveryWidth) {
|
||||
ExpectRoundTrip<uint8_t>(0x12, 0xf0);
|
||||
ExpectRoundTrip<uint16_t>(0x1234, 0xf00d);
|
||||
ExpectRoundTrip<uint32_t>(0x12345678u, 0xf00dbeefu);
|
||||
ExpectRoundTrip<uint64_t>(0x1234567890abcdefull, 0xf00dbeefcafef00dull);
|
||||
ExpectRoundTrip<int>(-7, 9);
|
||||
ExpectRoundTrip<double>(-2.5, 1e300);
|
||||
}
|
||||
|
||||
// fetch_add across the integer widths.
|
||||
TEST(Utils, AtomicRefFetchAddEveryWidth) {
|
||||
uint8_t u8 = 250;
|
||||
EXPECT_EQ(AtomicRef<uint8_t>(u8).fetch_add(3), 250);
|
||||
EXPECT_EQ(u8, 253);
|
||||
|
||||
uint16_t u16 = 65000;
|
||||
EXPECT_EQ(AtomicRef<uint16_t>(u16).fetch_add(500), 65000);
|
||||
EXPECT_EQ(u16, 65500);
|
||||
|
||||
int32_t i32 = -5;
|
||||
EXPECT_EQ(AtomicRef<int32_t>(i32).fetch_add(12), -5);
|
||||
EXPECT_EQ(i32, 7);
|
||||
|
||||
uint64_t u64 = 1ull << 40;
|
||||
EXPECT_EQ(AtomicRef<uint64_t>(u64).fetch_add(1), 1ull << 40);
|
||||
EXPECT_EQ(u64, (1ull << 40) + 1);
|
||||
}
|
||||
|
||||
// Every memory order the call sites use has to round-trip, including the
|
||||
// relaxed store in CalculateNormals/SortVerts and the acquire load in
|
||||
// HashTableD's probes.
|
||||
TEST(Utils, AtomicRefLoadStoreRoundTrip) {
|
||||
int value = 0;
|
||||
AtomicRef<int> ref(value);
|
||||
|
||||
ref.store(1, std::memory_order_relaxed);
|
||||
EXPECT_EQ(ref.load(std::memory_order_relaxed), 1);
|
||||
|
||||
ref.store(2, std::memory_order_release);
|
||||
EXPECT_EQ(ref.load(std::memory_order_acquire), 2);
|
||||
|
||||
ref.store(3);
|
||||
EXPECT_EQ(ref.load(), 3);
|
||||
EXPECT_EQ(value, 3);
|
||||
|
||||
// What HashTableD's const probes do: cast away const at the call site.
|
||||
const int& constValue = value;
|
||||
EXPECT_EQ(AtomicRef<int>(const_cast<int&>(constValue))
|
||||
.load(std::memory_order_acquire),
|
||||
3);
|
||||
}
|
||||
|
||||
// std::atomic_ref's mutators are const - it is a handle, so constness of the
|
||||
// handle says nothing about the referent. The C++17 stand-in has to match, or
|
||||
// code written against one standard fails to build on the other.
|
||||
TEST(Utils, AtomicRefMutatorsAreConst) {
|
||||
int value = 0;
|
||||
const AtomicRef<int> ref(value);
|
||||
ref.store(4);
|
||||
EXPECT_EQ(ref.load(), 4);
|
||||
EXPECT_EQ(ref.exchange(5), 4);
|
||||
int expected = 5;
|
||||
EXPECT_TRUE(ref.compare_exchange_strong(expected, 6));
|
||||
EXPECT_EQ(ref.fetch_add(1), 6);
|
||||
EXPECT_EQ(value, 7);
|
||||
}
|
||||
Reference in New Issue
Block a user