couple of questions about stockfish code ?

[BeyondCritics]

This is correct, and someone aware of these issues could have circumvented them easily. Nevertheless i assume it works on any platform with two's complement integer representation, which encompasses ARM and Intel processors. The point here is the difference between "undefined behaviour" and "implementation defined behaviour".

The real issue here is the abuse of the union and this _is_ serious "undefined behaviour". It is a common anti pattern, which happens to work on older, less optimizing compilers.

[syzygy]

This is correct, and someone aware of these issues could have circumvented them easily. Nevertheless i assume it works on any platform with two's complement integer representation, which encompasses ARM and Intel processors. The point here is the difference between "undefined behaviour" and "implementation defined behaviour".

The real issue here is the abuse of the union and this _is_ serious "undefined behaviour". It is a common anti pattern, which happens to work on older, less optimizing compilers.

The use of a union is actually the right way to do this without introducing undefined behaviour (as opposed to recasting pointers, violating aliasing rules).
The use of a union to map, in this case, between two 16-bits ints and one 32-bit int is not "undefined behaviour" but again "implementation-defined behaviour".

[syzygy]

This is correct, and someone aware of these issues could have circumvented them easily. Nevertheless i assume it works on any platform with two's complement integer representation, which encompasses ARM and Intel processors. The point here is the difference between "undefined behaviour" and "implementation defined behaviour".

The real issue here is the abuse of the union and this _is_ serious "undefined behaviour". It is a common anti pattern, which happens to work on older, less optimizing compilers.

The use of a union is actually the right way to do this without introducing undefined behaviour (as opposed to recasting pointers, violating aliasing rules).
The use of a union to map, in this case, between two 16-bits ints and one 32-bit int is not "undefined behaviour" but again "implementation-defined behaviour".

Hmm, it is certainly perfectly legal in C99 and C11.

In C++ it might be formally undefined, but at least g++ allows it as a language-extension. I'm sure Clang then does the same.

[Sven]

This is correct, and someone aware of these issues could have circumvented them easily. Nevertheless i assume it works on any platform with two's complement integer representation, which encompasses ARM and Intel processors. The point here is the difference between "undefined behaviour" and "implementation defined behaviour".

The real issue here is the abuse of the union and this _is_ serious "undefined behaviour". It is a common anti pattern, which happens to work on older, less optimizing compilers.

The use of a union is actually the right way to do this without introducing undefined behaviour (as opposed to recasting pointers, violating aliasing rules).
The use of a union to map, in this case, between two 16-bits ints and one 32-bit int is not "undefined behaviour" but again "implementation-defined behaviour".

Hmm, it is certainly perfectly legal in C99 and C11.

In C++ it might be formally undefined, but at least g++ allows it as a language-extension. I'm sure Clang then does the same.

All that trouble would be avoided by using a struct of two 16-bit integers ... And the speed difference between "struct" and the current "sophisticated" version (if there is any speed difference at all) would be negligible and not relevant for strength. You have to consider more than just the effort for adding two 32-bit integers vs. twice adding two 16-bit integers, there is also the additional decoding effort in mg_value().

[syzygy]

Addition of score values is far more frequent than encoding/decoding. I'm pretty sure using a struct of two 16-bit ints would be measurably slower. Also because I have no doubt that if there were no speed penalty, Stockfish would already use the "simpler" approach. (And probably it started with such an implementation, but I have not looked into what very old versions did.)

Anyway, why are people getting excited about this?

[Fulvio]

Addition of score values is far more frequent than encoding/decoding. I'm pretty sure using a struct of two 16-bit ints would be measurably slower.

Maybe you should read this:
https://en.wikipedia.org/wiki/Instructi ... arallelism
https://software.intel.com/en-us/blogs/ ... ndy-bridge

[syzygy]

Addition of score values is far more frequent than encoding/decoding. I'm pretty sure using a struct of two 16-bit ints would be measurably slower.

Maybe you should read this:
https://en.wikipedia.org/wiki/Instructi ... arallelism
https://software.intel.com/en-us/blogs/ ... ndy-bridge

Or maybe you should just test and measure.

Why do you think that using TWO registers to keep track of the aggregated score in evaluate() instead of just one incurs no performance penalty?

The only sensible alternative to the union approach that I see is wrapping the two 16-bit ints into one 32-bit int "by hand" using a shift and an OR/addition. This should in principle give more or less the same code (depending on the endianness of the platform). I assume it has been found out in the past that compilers did not handle this as efficiently (but I am no encyclopedia of Stockfish development... just answering a simple question that was posed here and being a little surprised with the shock and horror it is apparently causing).

[BeyondCritics]

...
The use of a union is actually the right way to do this without introducing undefined behaviour (as opposed to recasting pointers, violating aliasing rules).
...

The right way to do this, is not to break (or burden, for that matter) the type system. This means here to use shift instructions. On any modern compiler you will get perfectly optimized code from that, without any hassles or concerns.

[syzygy]

The only sensible alternative to the union approach that I see is wrapping the two 16-bit ints into one 32-bit int "by hand" using a shift and an OR/addition. This should in principle give more or less the same code (depending on the endianness of the platform).

This is actually what the new code is doing, and it uses a union only for converting between signed and unsigned 16-bits.

It seems to me this relies on implementation-defined behaviour as much as using straightforward casts. But I might be missing something here.

[syzygy]

...
The use of a union is actually the right way to do this without introducing undefined behaviour (as opposed to recasting pointers, violating aliasing rules).
...

The right way to do this, is not to break (or burden, for that matter) the type system. This means here to use shift instructions. On any modern compiler you will get perfectly optimized code from that, without any hassles or concerns.

Well, I was reacting to your allegation of abuse and undefined behaviour. That may hold some truth for C++, it seems, but not for C99/C11 and not for C++ as "extended" by g++.

I do agree that SF's current approach is to be preferred as it does not rely on endianness.