coolfluid / coolfluid3

COOLFluiD is a collaborative simulation environment focused on complex multi-physics simulations
http://coolfluid.github.com
76 stars 77 forks source link

GCC4.1 release build needs -O2 #195

Closed barche closed 12 years ago

barche commented 12 years ago

Due to a bug in GCC 4.1, Eigen will not function properly in -O3 mode. We should adapt the build system so the release flags are set to -O2 when GCC 4.1 is used.

See http://eigen.tuxfamily.org/bz/show_bug.cgi?id=424

tlmquintino commented 12 years ago

Funny enough I noticed a very similar problem with some code at work. A tool that we have uses some code generated by byson/flex. When we use it with GCC4.1 with -O3 it somehow optimizes out code that is useful - and then it simply does not call a function that we need.

We solved this in cmake by enforcing that with GCC we always downgrade the compilation to -O2.

We also don't use the default "Release" build type, and created a build type "Production" that does the above.

barche commented 12 years ago

OK, we now check for the version of GCC, and in the case of 4.1 the O3 is set to O2. For now, the check is based on http://stackoverflow.com/questions/4058565/check-gcc-minor-in-cmake but apparently starting with cmake 2.8.8 the variable CMAKE_CXX_COMPILER_VERSION will be introduced, so we might use that in a year or so.

tlmquintino commented 12 years ago

Honestly, irrespectively of the version of GCC I would always downgrade -O3 to -O2. If you read the man page, gcc does not guarantee that O2 will provide better optimizations than O3. In fact it may well turn out to be slower, because of it may generate bigger byte-code which in a tight loop degrade performance also.

barche commented 12 years ago

Well, just looking at the assembly for the cylinder2d test, I got this with O2: SpecializedAssembly: mean: 0.0168099, min: 0.0139175, max: 0.0192885 SpecializedAssembly: mean: 0.0167811, min: 0.0139392, max: 0.0187285 and with O3: SpecializedAssembly: mean: 0.0158646, min: 0.0132455, max: 0.017833 SpecializedAssembly: mean: 0.0159907, min: 0.0133548, max: 0.0181696

So I agree, the difference is minimal, but since we can expect a small gain, and the change is basically free, and O3 seems to be what CMake sets as default for release anyway, I feel safer sticking with O3 for Release builds.

tlmquintino commented 12 years ago

One point on the graph does not make a trend ;) OK, not to create surprises, lets keep the -O3 as standard in Release, but I think in this particular case, CMake went a bit beyond the line.