From a2f5b635d130040e2a7b10bf0e2430a4e01dacfb Mon Sep 17 00:00:00 2001 From: Mathias Agopian Date: Tue, 30 Jul 2024 09:39:09 -0700 Subject: [PATCH] don't mix seq_cst with other memory orders this change shouldn't have any impact on ARM, however, according to cppreference it's not safe to mix seq_cst with other memory orders: "as soon as atomic operations that are not tagged memory_order_seq_cst enter the picture, the sequential consistency guarantee for the program is lost" --- libs/utils/include/utils/WorkStealingDequeue.h | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/libs/utils/include/utils/WorkStealingDequeue.h b/libs/utils/include/utils/WorkStealingDequeue.h index f9f5b5fbc2..43735f0a57 100644 --- a/libs/utils/include/utils/WorkStealingDequeue.h +++ b/libs/utils/include/utils/WorkStealingDequeue.h @@ -92,9 +92,11 @@ void WorkStealingDequeue::push(TYPE item) noexcept { index_t bottom = mBottom.load(std::memory_order_relaxed); setItemAt(bottom, item); - // std::memory_order_release is used because we release the item we just pushed to other + // Here we need std::memory_order_release because we release, the item we just pushed, to other // threads which are calling steal(). - mBottom.store(bottom + 1, std::memory_order_release); + // However, generally seq_cst cannot be mixed with other memory orders. So we must use seq_cst. + // see: https://plv.mpi-sws.org/scfix/paper.pdf + mBottom.store(bottom + 1, std::memory_order_seq_cst); } /* @@ -154,9 +156,11 @@ TYPE WorkStealingDequeue::pop() noexcept { assert(top - bottom == 1); } - // std::memory_order_relaxed used because we're not publishing any data. + // Here, we only need std::memory_order_relaxed because we're not publishing any data. // No concurrent writes to mBottom possible, it's always safe to write mBottom. - mBottom.store(top, std::memory_order_relaxed); + // However, generally seq_cst cannot be mixed with other memory orders. So we must use seq_cst. + // see: https://plv.mpi-sws.org/scfix/paper.pdf + mBottom.store(top, std::memory_order_seq_cst); return item; }