bug #1572: use c++11 atomic instead of volatile if c++11 is available, and disable multi-threaded GEMM on non-x86 without c++11.

This commit is contained in:
Gael Guennebaud
2018-07-17 00:11:20 +02:00
parent add5757488
commit 40797dbea3
5 changed files with 34 additions and 7 deletions

View File

@@ -108,7 +108,7 @@ static void run(Index rows, Index cols, Index depth,
// i.e., we test that info[tid].users equals 0.
// Then, we set info[tid].users to the number of threads to mark that all other threads are going to use it.
while(info[tid].users!=0) {}
info[tid].users += threads;
info[tid].users = threads;
pack_lhs(blockA+info[tid].lhs_start*actual_kc, lhs.getSubMapper(info[tid].lhs_start,k), actual_kc, info[tid].lhs_length);
@@ -146,7 +146,9 @@ static void run(Index rows, Index cols, Index depth,
// Release all the sub blocks A'_i of A' for the current thread,
// i.e., we simply decrement the number of users by 1
for(Index i=0; i<threads; ++i)
#if !EIGEN_HAS_CXX11_ATOMIC
#pragma omp atomic
#endif
info[i].users -= 1;
}
}

View File

@@ -10,6 +10,10 @@
#ifndef EIGEN_PARALLELIZER_H
#define EIGEN_PARALLELIZER_H
#if EIGEN_HAS_CXX11_ATOMIC
#include <atomic>
#endif
namespace Eigen {
namespace internal {
@@ -75,8 +79,17 @@ template<typename Index> struct GemmParallelInfo
{
GemmParallelInfo() : sync(-1), users(0), lhs_start(0), lhs_length(0) {}
// volatile is not enough on all architectures (see bug 1572)
// to guarantee that when thread A says to thread B that it is
// done with packing a block, then all writes have been really
// carried out... C++11 memory model+atomic guarantees this.
#if EIGEN_HAS_CXX11_ATOMIC
std::atomic<Index> sync;
std::atomic<int> users;
#else
Index volatile sync;
int volatile users;
#endif
Index lhs_start;
Index lhs_length;
@@ -87,7 +100,10 @@ void parallelize_gemm(const Functor& func, Index rows, Index cols, Index depth,
{
// TODO when EIGEN_USE_BLAS is defined,
// we should still enable OMP for other scalar types
#if !(defined (EIGEN_HAS_OPENMP)) || defined (EIGEN_USE_BLAS)
// Without C++11, we have to disable GEMM's parallelization on
// non x86 architectures because there volatile is not enough for our purpose.
// See bug 1572.
#if (! defined(EIGEN_HAS_OPENMP)) || defined(EIGEN_USE_BLAS) || ((!EIGEN_HAS_CXX11_ATOMIC) && !(EIGEN_ARCH_i386_OR_x86_64))
// FIXME the transpose variable is only needed to properly split
// the matrix product when multithreading is enabled. This is a temporary
// fix to support row-major destination matrices. This whole

View File

@@ -537,7 +537,7 @@
#endif
#endif
// Does the compiler support type_trais?
// Does the compiler support type_traits?
#ifndef EIGEN_HAS_TYPE_TRAITS
#if EIGEN_MAX_CPP_VER>=11 && (EIGEN_HAS_CXX11 || EIGEN_COMP_MSVC >= 1700)
#define EIGEN_HAS_TYPE_TRAITS 1
@@ -617,6 +617,16 @@
#endif
#endif
#ifndef EIGEN_HAS_CXX11_ATOMIC
#if EIGEN_MAX_CPP_VER>=11 && \
(__has_feature(cxx_atomic) \
|| (__cplusplus > 201103L) \
|| ((__cplusplus >= 201103L) && (EIGEN_COMP_MSVC==0 || EIGEN_COMP_MSVC >= 1700)))
#define EIGEN_HAS_CXX11_ATOMIC 1
#else
#define EIGEN_HAS_CXX11_ATOMIC 0
#endif
#endif
#if defined(EIGEN_CUDACC) && EIGEN_HAS_CONSTEXPR
// While available already with c++11, this is useful mostly starting with c++14 and relaxed constexpr rules