New user self-registration is disabled due to spam. Please email eigen-core-team @ lists.tuxfamily.org if you need an account.
Bug 1369 - silence compiler warnings due to type mismatch (GeneralMatrixMatrix.h, Parallelizer.h)
Summary: silence compiler warnings due to type mismatch (GeneralMatrixMatrix.h, Parall...
Status: RESOLVED FIXED
Alias: None
Product: Eigen
Classification: Unclassified
Component: Core - matrix products (show other bugs)
Version: unspecified
Hardware: All All
: Normal Unknown
Assignee: Nobody
URL: https://github.com/jeffhammond/eigen/...
Whiteboard:
Keywords:
Depends on:
Blocks: 3.3
  Show dependency treegraph
 
Reported: 2016-12-27 23:35 UTC by Jeff Hammond
Modified: 2017-01-24 08:20 UTC (History)
3 users (show)



Attachments
Git patch file (2.08 KB, patch)
2016-12-27 23:36 UTC, Jeff Hammond
no flags Details | Diff

Description Jeff Hammond 2016-12-27 23:35:38 UTC
I apologize for this, but I have not yet reminded myself how to use Mercurial.  I have a trivial (four line) patch against the Github mirror, which anyone can reproduce in seconds.

See https://github.com/jeffhammond/eigen/commit/1f9f77b1dfa3a74ad81ac2b649a3740ac35ec1b2 or below (from git format-patch 50015ab88ef680617747a3c25e956064b07430a9 && cat 0001-silence-compiler-warnings-due-to-type-mismatch.patch) for details.

From 1f9f77b1dfa3a74ad81ac2b649a3740ac35ec1b2 Mon Sep 17 00:00:00 2001
From: Jeff Hammond <jeff.science@gmail.com>
Date: Wed, 21 Dec 2016 21:51:17 -0800
Subject: [PATCH] silence compiler warnings due to type mismatch

1) omp thread ids are always int, so the return values of
   the associated runtime functions should be stored in
   int values, not Index values.

2) because sync and users are compared or stored to from Index,
   they need to be Index, not int, because Index can (and is in my case)
   a ptrdiff_t, which is a 64b type (when int is a 32b type).

It is not clear why Index is not an int, but presumably Eigen supports
matrices of rank greater than INT_MAX.

The use of volatile qualifier on sync suggests unsafe assumptions about
how inter-thread synchronization works, but that is a problem for
another day.
---
 Eigen/src/Core/products/GeneralMatrixMatrix.h | 4 ++--
 Eigen/src/Core/products/Parallelizer.h        | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/Eigen/src/Core/products/GeneralMatrixMatrix.h b/Eigen/src/Core/products/GeneralMatrixMatrix.h
index 61df3be57..75e56ea22 100644
--- a/Eigen/src/Core/products/GeneralMatrixMatrix.h
+++ b/Eigen/src/Core/products/GeneralMatrixMatrix.h
@@ -83,8 +83,8 @@ static void run(Index rows, Index cols, Index depth,
   if(info)
   {
     // this is the parallel version!
-    Index tid = omp_get_thread_num();
-    Index threads = omp_get_num_threads();
+    int tid = omp_get_thread_num();
+    int threads = omp_get_num_threads();
 
     LhsScalar* blockA = blocking.blockA();
     eigen_internal_assert(blockA!=0);
diff --git a/Eigen/src/Core/products/Parallelizer.h b/Eigen/src/Core/products/Parallelizer.h
index 2a31e4cbe..4b4ca669a 100644
--- a/Eigen/src/Core/products/Parallelizer.h
+++ b/Eigen/src/Core/products/Parallelizer.h
@@ -75,8 +75,8 @@ template<typename Index> struct GemmParallelInfo
 {
   GemmParallelInfo() : sync(-1), users(0), lhs_start(0), lhs_length(0) {}
 
-  int volatile sync;
-  int volatile users;
+  Index volatile sync;
+  Index volatile users;
 
   Index lhs_start;
   Index lhs_length;
-- 
2.11.0
Comment 1 Jeff Hammond 2016-12-27 23:36:27 UTC
Created attachment 764 [details]
Git patch file
Comment 2 Gael Guennebaud 2016-12-28 22:38:00 UTC
Thanks for the patch, I applied slightly fixes though:

https://bitbucket.org/eigen/eigen/commits/ec0f519aaaec/
https://bitbucket.org/eigen/eigen/commits/b41d7cc81e72/

I'll wait a bit before backporting them and closing this entry.

Note You need to log in before you can comment on or make changes to this bug.