This bugzilla service is closed. All entries have been migrated to https://gitlab.com/libeigen/eigen
Bug 639 - OpenMP GEMM kernel: possible truncation from long int (Index) to int
Summary: OpenMP GEMM kernel: possible truncation from long int (Index) to int
Status: NEW
Alias: None
Product: Eigen
Classification: Unclassified
Component: Core - matrix products (show other bugs)
Version: 3.2
Hardware: All All
: Normal minor
Assignee: Nobody
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-08-06 17:37 UTC by Christian Seiler
Modified: 2019-12-04 12:32 UTC (History)
0 users



Attachments

Description Christian Seiler 2013-08-06 17:37:35 UTC
I like to compile my software with -Wconversion and -Werror while I am developing it. Unfortunately, when compiling with OpenMP, I get the following warning messages (that become errors with -Werror) in the parallelized GEMM kernel:

[ ... lots of template instantiations left out ... ]
Eigen/src/Core/products/GeneralMatrixMatrix.h:107:7: error: conversion to ‘int’ from ‘long int’ may alter its value [-Werror=conversion]
Eigen/src/Core/products/GeneralMatrixMatrix.h:112:7: error: conversion to ‘int’ from ‘long int’ may alter its value [-Werror=conversion]

(long int corresponds to the default Index on my system, int is the data type of the 'users' and 'sync' fields in the GemmParallelInfo structure.)

Eigen Version: 3.2.0
Compiler: g++ 4.6.2
(Hardware is x86-64 and OS is Linux, but I selected 'All' in the bug report because this problem should be generic.)

Reproduce with:
--------------------------------------------------------
#include <Eigen/Core>
#include <iostream>

int main()
{
  Eigen::MatrixXd one = Eigen::MatrixXd::Random(8, 8);
  Eigen::MatrixXd two = Eigen::MatrixXd::Random(8, 8);
  std::cout << (one * two) << std::endl;
  return 0;
}
--------------------------------------------------------

Compiler flags:
g++ -fopenmp -I../Eigen3.2 -Wall -Wconversion -Werror -o test test.cpp
No error on:
g++ -I../Eigen3.2 -Wall -Wconversion -Werror -o test test.cpp

I don't completely understand the parallelized version, but from what I gather from the source code, the correct solution would probably be:

 - leave 'users' as int and change the 'threads' variable,
   GeneralMatrixMatrix.c:81, to type int, since omp_get_num_threads() always
   returns int anyway (and more than 2^31 threads is really unlikely, even in
   the foreseeable future), and possible add some appropriate casts at the
   places where the compiler due to -Wconversion still doesn't like it
   (Probably wise to also test with int16_t as Index type for this.)
 - change 'sync' member to Index, since the 'depth' comes from matrix sizes
   (there's the caveat that since it's volatile for sync purposes, there
   should be some kind of guarantee that the processor will read it either
   in it's entirety; this could be an issue if Index is larger than the word
   size (int) and the value of depth is also larger than the maximum
   representable integer)

For any practical purposes, it also may be enough to just add casts on the two lines where the currrent warning message is generated, since general matrix-matrix multiplications with sizes larger than 2^31 are so expensive that nobody ever does them. (On the other hand, they'd be a prime candidate for parallelisation on systems with a huge amount (>1000) cores.)
Comment 1 Nobody 2019-12-04 12:32:04 UTC
-- GitLab Migration Automatic Message --

This bug has been migrated to gitlab.com's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.com/libeigen/eigen/issues/639.

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