G++ Broken (?) 5.0 through 6.2. Fixed in 6.3 and later.

[paboyle]

https://wandbox.org/permlink/tzssJza6R9XnqANw https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80652

Getting Travis fails under gcc-5 for Test_simd, now that I added more comprehensive testing to the CI test suite. The limitations of Travis runtime limits & weak cores are being shown.

Travis uses 5.4.1 for g++-5.

We are going to move to a new CI server we bought for the purpose soon.

Working (-) Broken (X): 4.9.0 - 4.9.1 - 5.1.0 X 5.2.0 X 5.3.0 X 5.4.0 X 6.1.0 X 6.2.0 X 6.3.0 - 7.1.0 - 8.0.0 (HEAD) -

Sigh.

Options: a) Drop to -O2 under broken G++ versions b) Refuse to build with broken G++ versions.

Opinions sought.

[paboyle]

Attempting to work around with

if (GNUC == 5 ) || ( ( GNUC == 6 ) && __GNUC_MINOR__ < 3 )

pragma GCC push_options

pragma GCC optimize ("O0")

endif

and same to pop options around the SimdApply in Grid_vector_types.

But, I now have very, very, VERY poor confidence in these compiler versions.

e.g. Where else do we hit this? It is dangerous to not apply this globally (which we could force) but that will cripple performance.

Do we unsupport a whole swathe of G++ versions?

I posted on stack overflow to try to get a double check on the legality of the union use.

http://stackoverflow.com/questions/2906365/gcc-strict-aliasing-and-casting-through-a-union/43820916#43820916

But, I think this is legal code.

[paboyle]

Clang 3.5 through 5.0 are good on this.

[paboyle]

Just a note; I froze 0.7.0 and put in a force drop of optimisation levels on the broken G++ versions We can replace with a patch to refuse compile if the majority vote is for that.

Borken G++ versions pass Grid self tests with the optimisation drop. I've had two emails from G++ bug mailing list but no acknowledge of bug so far (1 query, one expression of interest).

Seems the pragmatic solution until we know more about compiler breakage or not.

[paboyle]

As far as I know g++ 4.8 might have breakage. I recall it does; will check. The versions I presently know work are 4.9 . 6.3 and later.

Solution, avoiding a big verbose issues, is to place a single header, included ONLY from Init.cc direct:

error on <= 4.8 silent on == 4.9 warning on 5.0 through 6.2 silent on >=6.3

The error/warn will be triggered only once during the compile

[paboyle]

p.s. if anyone has used 4.8 recently and succeeded please post immediately !!!

[coppolachan]

I would say to drop completely the support for gcc <4.8.1, they are too old and with no full support of c++11. We can be drastic and drop the support for the 4.8 series completely.

Notice that we should issue an error also for Intel 16.0.2 where we trigger a bug of the compiler.

[paboyle]

This is the last open issue for 0.7.0 being closed. I will recheck head on the ICC KNL system at BNL and then close the release.

• Clang >= 3.5 required.

• ICC > 16.0.2 required.

• 4.8 is ruled out. As I said, compiler is broken. I rechecked.

• 5.x, 6.0-2 gives a warn: I have retained the pragma forcing -O0 for GCC on the relevant conversion functions that trip the error.

  CXX util/Init.o In file included from ../../lib/util/Init.cc:51:0: /home/travis/build/paboyle/Grid/include/Grid/util/CompilerCompatible.h:39:8: warning: #warning "g++ version 5 is known to not work with Grid due to compiler bugs under -O3 : ensure you run make check" [-Wcpp]

  warning "g++ version 5 is known to not work with Grid due to compiler bu

  ^

  CXX stencil/Lebesgue.o

[paboyle]

I've read more and appears to be my misunderstanding of the standard.

This is a terrifying difference in the meaning of "union" between "C" and more recent "C++" standards, that only GCC implements.

[paboyle]

Response is now clear. Audit and elimate ALL use of union in the code base.

Eigen is also doing this.

./lib/Eigen/src/Core/arch/AltiVec/PacketMath.h: union { ./lib/Eigen/src/Core/arch/AltiVec/PacketMath.h: union { ./lib/Eigen/src/Core/arch/AltiVec/PacketMath.h: union { ./lib/Eigen/src/Core/arch/AltiVec/PacketMath.h: union { ./lib/Eigen/src/Core/arch/AltiVec/PacketMath.h: union { ./lib/Eigen/src/Core/arch/AltiVec/PacketMath.h: union { ./lib/Eigen/src/Core/arch/CUDA/Half.h:union FP32 { ./lib/Eigen/src/Core/arch/ZVector/Complex.h: union { ./lib/Eigen/src/Core/arch/ZVector/PacketMath.h:typedef union { ./lib/Eigen/src/Geometry/AlignedBox.h: /* Returns an AlignedBox that is the union of \a b and \c this. ./lib/Eigen/src/MetisSupport/MetisSupport.h: // Compute the union structure of of A(j,:) and At(j,:) ./lib/Eigen/src/OrderingMethods/Eigen_Colamd.h: union ./lib/Eigen/src/OrderingMethods/Eigen_Colamd.h: union ./lib/Eigen/src/OrderingMethods/Eigen_Colamd.h: union ./lib/Eigen/src/OrderingMethods/Eigen_Colamd.h: union ./lib/Eigen/src/OrderingMethods/Eigen_Colamd.h: union ./lib/Eigen/src/OrderingMethods/Eigen_Colamd.h: union ./lib/Eigen/src/OrderingMethods/Eigen_Colamd.h: / pivot row is the union of all rows in the pivot column pattern / ./lib/Eigen/src/OrderingMethods/Eigen_Colamd.h: / make the score the external degree of the union-of-rows / ./lib/Eigen/src/SparseCore/SparseColEtree.h: { // A sequence of interleaved find and union is performed ./lib/Eigen/src/SuperLUSupport/SuperLUSupport.h: union {int nnz;int lda;};

Grid's internal exposure is limited to lib/simd.

./lib/simd/Grid_avx.h: union uconv { ./lib/simd/Grid_avx.h: union u256f { ./lib/simd/Grid_avx.h: union u256d { ./lib/simd/Grid_avx512.h: union u512f { ./lib/simd/Grid_avx512.h: union u512d { ./lib/simd/Grid_neon.h: union uconv { ./lib/simd/Grid_neon.h: union u128f { ./lib/simd/Grid_neon.h: union u128d { ./lib/simd/Grid_sse4.h: union uconv { ./lib/simd/Grid_sse4.h: union u128f { ./lib/simd/Grid_sse4.h: union u128d { ./lib/simd/Grid_sse4.h: union FP32 { ./lib/simd/Grid_vector_types.h: typedef union conv_t_union { ./lib/simd/Grid_vector_types.h: conv_t_union(){};

[paboyle]

Got this back from pinskia at gcc dot gnu.org

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80652

--- Comment #7 from Andrew Pinski --- (In reply to Peter Boyle from comment #6) Just a comment -- suggest a warning thrown if union access from two views is made. AFAIK g++ is the only compiler not implementing the defacto type pun use.

http://en.cppreference.com/w/cpp/language/union

"Many compilers implement, as a non-standard language extension, the ability to read inactive members of a union."

Actually it does except you are accessing not directly from the union but rather than via a pointer (reference) which is where the problem comes in. Just change: "const scalar &a" to be "const scalar a" and you see the behavior you originally expected.

[coppolachan]

So the suggested solution is changing all the functors to pass arguments by value?

[paboyle]

No, better to locally make the change in "SimdApply" by copying out of the union into a "scalar_type s" which is "safe" to pass by reference or not to any functor we choose.

Personally, I think given replies from G++ team that they support such union use, I think this is still a bug. If we can access it, as a scalar type, why not pass as const by reference...

I've committed a change to develop and this passed the Test_simd under g++ 5.4 on Travis.

[coppolachan]

Ok let's check if this is going to impact the performance in some way.

[giltirn]

I think it may also be worth revisiting my concerns about casting vector pointers to scalar pointers directly (ie not through a union), an example of which can be found in pokeLocalSite:

   scalar_type * vp = (scalar_type *)&l._odata[odx];  //l is Lattice 

object scalar_type pt = (scalar_type )&s;

   for(int w=0;w<words;w++){
     vp[idx+w*Nsimd] = pt[w];
   }

If this is legal then perhaps we can avoid using unions?

On 05/17/2017 07:05 AM, Peter Boyle wrote:

No, better to locally make the change in "SimdApply" by copying out of the union into a "scalar_type s" which is "safe" to pass by reference or not to any functor we choose.

Personally, I think given replies from G++ team that they support such union use, I think this is still a bug. If we can access it, as a scalar type, why not pass as const by reference...

I've committed a change to develop and this passed the Test_simd under g++ 5.4 on Travis.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/paboyle/Grid/issues/100#issuecomment-302057733, or mute the thread https://github.com/notifications/unsubscribe-auth/AIitA4vWPF7aEccV9h-beHyPUE7-eV0aks5r6tRjgaJpZM4NSw9N.

[paboyle]

Claims exit that GCC 9.3 fills memory and fails to compile.