This bugzilla service is closed. All entries have been migrated to https://gitlab.com/libeigen/eigen
Bug 1674 - SIMD sin/cos gives wrong results with -ffast-math
Summary: SIMD sin/cos gives wrong results with -ffast-math
Status: RESOLVED FIXED
Alias: None
Product: Eigen
Classification: Unclassified
Component: Core - general (show other bugs)
Version: 3.4 (development)
Hardware: x86 - 64-bit Linux
: Normal Wrong Result
Assignee: Nobody
URL:
Whiteboard:
Keywords:
Depends on:
Blocks: 3.4
  Show dependency treegraph
 
Reported: 2019-02-01 18:22 UTC by william.tambellini
Modified: 2019-12-04 18:27 UTC (History)
4 users (show)



Attachments

Description william.tambellini 2019-02-01 18:22:45 UTC
cos/unaryExpr dont seem to work any more on master (eigen-24d29254152d) at least in release build (O3) with gcc6.1, no avx, just SSE:

// T = float
Eigen::Tensor<T, 2> i(2, 5);
i.setValues({{-0.5, -3.0f / 8.0f, -0.25f, 0f, 1.0f / 6.0f}, {0.25f, 1.f / 3.f, 0.5f, 0.7f, 1f}});
std::cout << "input:\n" << i << std::endl;
Eigen::Tensor<T, 2> i2 = i * (T)M_PI;
std::cout << "input*pi:\n" << i2 << std::endl;
std::cout << "cos tensor:\n" << i2.unaryExpr(Eigen::internal::scalar_cos_op<T>()) << std::endl;
using CMMatrix = Eigen::Matrix<T, Eigen::Dynamic, Eigen::Dynamic, Eigen::ColMajor>;
std::cout << "cos matrix:\n" << Eigen::Map<CMMatrix>(i2.data(), 2, 5).array().cos() << std::endl;
std::cout << "expected:\n";
for (unsigned i = 0; i < i2.dimension(0); ++i) {
for (unsigned j = 0; j < i2.dimension(1); ++j)
std::cout << std::setw(4) << std::cos(i2(i, j)) << " ";
std::cout << std::endl;
}

input:
-0.5 -0.375 -0.25 0 0.166667
0.25 0.333333 0.5 0.7 1
input*pi:
-1.5708 -1.1781 -0.785398 0 0.523599
0.785398 1.0472 1.5708 2.19911 3.14159
cos tensor:
-0 -0 1 1 0.866025
1 -0 -0 -0 -1
cos matrix:
-0 -0 1 1 0.866025
1 -0 -0 -0 -1
expected:
-4.37114e-08 0.382683 0.707107 1 0.866025
0.707107 0.5 -4.37114e-08 -0.587785 -1 

The results are correct in debug build but not release.
Could anyone at least try that repro ?
Comment 1 Gael Guennebaud 2019-02-02 12:28:00 UTC
I cannot reproduce. I tried various compilation options (including -O3 -DNDEBUG) and various compilers (including gcc 6.5).

Try to update your compiler to 6.5 and share your exact compilation options. Does it fails for you in isolation?


-------



#include <iostream>
#include <iomanip>
#include <unsupported/Eigen/CXX11/Tensor>

int main() {
typedef float T;
Eigen::Tensor<T, 2> i(2, 5);

i.setValues({{-0.5, -3.0f / 8.0f, -0.25f, 0.f, 1.0f / 6.0f}, {0.25f, 1.f / 3.f, 0.5f, 0.7f, 1.f}});

std::cout << "input:\n" << i << std::endl;
Eigen::Tensor<T, 2> i2 = i * (T)M_PI;
std::cout << "input*pi:\n" << i2 << std::endl;
std::cout << "cos tensor:\n" << i2.unaryExpr(Eigen::internal::scalar_cos_op<T>()) << std::endl;
using CMMatrix = Eigen::Matrix<T, Eigen::Dynamic, Eigen::Dynamic, Eigen::ColMajor>;
std::cout << "cos matrix:\n" << Eigen::Map<CMMatrix>(i2.data(), 2, 5).array().cos() << std::endl;
std::cout << "expected:\n";
for (unsigned i = 0; i < i2.dimension(0); ++i) {
for (unsigned j = 0; j < i2.dimension(1); ++j)
std::cout << std::setw(4) << std::cos(i2(i, j)) << " ";
std::cout << std::endl;
}

}
Comment 2 william.tambellini 2019-02-03 00:38:54 UTC
I confirm these lines fails but only in release, not debug, with gcc6.1 and these compile flags:
-Ieigen-24d29254152d -pthread -fPIC 
-march=corei7 -mno-fma4 -msse4.1 -msse4.2 -mpopcnt -mpclmul -mfpmath=sse
-std=c++14 
-O3 --param allow-store-data-races=1 
-fcx-limited-range -funsafe-math-optimizations -fopenmp -fno-keep-static-consts -fmerge-all-constants 
-DNDEBUG  
-falign-functions=1 -falign-loops=1 -falign-jumps=1 -falign-labels=1 -fomit-frame-pointer 
-fassociative-math -fno-signed-zeros -fno-trapping-math -fno-math-errno -freciprocal-math 
-fcx-limited-range -fno-signaling-nans -fno-rounding-math -funsafe-math-optimizations    
-fmax-errors=2 -ftemplate-backtrace-limit=0 
-o main.cpp.o -c main.cpp

Will try to repro with other versions of gcc.
Tks.
Comment 3 william.tambellini 2019-02-03 02:47:26 UTC
Got it : the culprit is 
-funsafe-math-optimizations
Comment 4 william.tambellini 2019-02-03 02:54:42 UTC
So strangely 
unsafe-math-optimizations
is not bothering eigen at least up to eigen-9fc878972a50
but then recently, it makes at least cos failing.
 
https://andrei600.wordpress.com/2016/03/15/gcc-compiler-options-funsafe-math-optimizations-and-ftracer/

I guess there is probably no way to detect that option at compilation time so no way for eigen to adjust/warn/assert ?
Comment 5 Gael Guennebaud 2019-02-03 07:56:45 UTC
Fixed for GCC:

https://bitbucket.org/eigen/eigen/commits/1b7242d240a1/
Summary:     Bug 1674: disable GCC's unsafe-math-optimizations in sin/cos vectorization (results are completely wrong otherwise)

but I also observe wrong results with clang >= 7 and:

__attribute__((optimize("...")))

is not supported by clang.
Comment 6 william.tambellini 2019-02-05 05:34:20 UTC
I confirm it now works at least for sin/cos. Tks.
Comment 7 Christoph Hertzberg 2019-02-22 13:11:55 UTC
To avoid associative-math optimization, we could place some strategical
  asm("" : "+x"(a));

Should work with any packet-type (and also works with GCC):
https://godbolt.org/z/0qWaMo
Comment 8 Gael Guennebaud 2019-02-22 14:45:59 UTC
This needs to be adapted for NEON, ALtivec, MSA, but fixing x86 targets should already be enough.
Comment 9 Gael Guennebaud 2019-02-22 14:54:11 UTC
Tested with SSE, AVX and FMA.

https://bitbucket.org/eigen/eigen/commits/21371d2bdadb/
Summary:     Bug 1674: workaround clang fast-math aggressive optimizations

Actually, it failed with clang 5 and 6 too, not only clang 7 (and probably older ones).

For ARM we could use 'w' instead of 'x' but I don't have clang+arm to check (only GCC+arm).

I would propose to close this one.
Comment 10 Nobody 2019-12-04 18:27:00 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/1674.

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