My experience with Linux/GCC

[rvida]

I just want to share my experience with porting from win/msvc to linux/gcc.

GCC with -O2 setting optimizes very aggressively. In most cases where it makes a potentially 'unsafe' assumption it gives at least a warning. I had to rewrite some code pieces due to pointer-aliasing but that was pretty straightforward.
The biggest headache was with the following code:

Code: Select all

inline Score eg_value(ScorePair s) {
  return Score(short(s & 0xffff));
}

inline Score mg_value(ScorePair s) {
  return Score((int(s) + 0x8000) >> 16);
}

MSVC (and GCC with -O1) does understand this, and correctly sign-extends by emmitting movsx/cwd/cdq instructions depending on context. However, GCC with -O2 completely optimized away eg_value(), and did some very weird things with mg_value().

In desperation, I took a look at StockFish's value.h and indeed found a solution therein (SF guys are much better at C/C++ stuff than me).
Now this code works correctly:

Code: Select all

#ifdef __GNUC__

inline Score eg_value(ScorePair s) {
  return Score((int)(unsigned(s) & 0x7fffu) - (int)(unsigned(s) & 0x8000u));
}

inline Score mg_value(ScorePair s) {
  return Score(((int(s) + 0x8000) & ~0xffff) / 0x10000);
}

#else

inline Score eg_value(ScorePair s) {
  return Score(short(s & 0xffff));
}

inline Score mg_value(ScorePair s) {
  return Score((int(s) + 0x8000) >> 16);
}

#endif

Big thanks to the StockFish team! I owe them some beer

Contrary to my fears, the threading, locking and signaling stuff wasn't very hard to port. In case someone finds it useful here is my smp.h:

Code: Select all

#pragma once

#ifdef _WIN32

#include <windows.h>

class Event {
  HANDLE hEvent;
public:
  inline Event() {};

  inline void init() { 
    hEvent = CreateEvent(NULL, false, false, NULL); 
  }

  inline void destroy() { 
    CloseHandle(hEvent); 
  }

  inline void signal() { 
    SetEvent(hEvent); 
  }

  inline void wait_for() { 
    WaitForSingleObject(hEvent, INFINITE); 
  }

  inline bool wait_for(int timeout) {
    return (WaitForSingleObject(hEvent, timeout) == WAIT_TIMEOUT);
  }

};

#define ThreadProcResult DWORD WINAPI
typedef LPVOID ThreadProcParam;
typedef ThreadProcResult ThreadProc(ThreadProcParam);

inline void start_thread(ThreadProc proc, ThreadProcParam param) {
  DWORD tid;
  CloseHandle(CreateThread(NULL, 0, proc, param, 0, &tid));
}

typedef CRITICAL_SECTION Lock;

#define lock_init(lock) InitializeCriticalSection(&(lock))
#define lock_acquire(lock) EnterCriticalSection(&(lock))
#define lock_release(lock) LeaveCriticalSection(&(lock))
#define lock_delete(lock) DeleteCriticalSection(&(lock))

#else

#include <pthread.h>
#include <errno.h>

class Event {
  pthread_cond_t cond;
  pthread_mutex_t mutex;
  volatile bool signaled;
public:
  inline Event() {};

  inline void init() {
    pthread_cond_init(&cond, NULL);
    pthread_mutex_init(&mutex, NULL);
    signaled = false;
  }

  inline void destroy() {
    pthread_cond_destroy(&cond);
    pthread_mutex_destroy(&mutex);
  }

  inline void signal() {
    pthread_mutex_lock(&mutex);
    pthread_cond_broadcast(&cond);
    signaled = true;
    pthread_mutex_unlock(&mutex);
  }

  inline void wait_for() {
    pthread_mutex_lock(&mutex);
    if (!signaled)
      pthread_cond_wait(&cond, &mutex);
    signaled = false;
    pthread_mutex_unlock(&mutex);
  }

  inline bool wait_for(int timeout) {
    pthread_mutex_lock(&mutex);
    int res = 0;
    if (!signaled) {
      struct timespec abstime;
      abstime.tv_sec  = timeout / 1000000;
      abstime.tv_nsec = timeout % 1000000;
      res = pthread_cond_timedwait(&cond, &mutex, &abstime);
    }
    signaled = false;
    pthread_mutex_unlock(&mutex);
    return (res == ETIMEDOUT);
  }
};

#define ThreadProcResult void *
typedef void * ThreadProcParam;
typedef ThreadProcResult ThreadProc(ThreadProcParam);

inline void start_thread(ThreadProc proc, ThreadProcParam param) {
  pthread_t pthread[1];
  pthread_attr_t attrib;
  pthread_attr_init(&attrib);
  pthread_create(pthread, &attrib, proc, param);
}

typedef pthread_mutex_t Lock;

#define lock_init(x) pthread_mutex_init(&(x), NULL)
#define lock_acquire(x) pthread_mutex_lock(&(x))
#define lock_release(x) pthread_mutex_unlock(&(x))
#define lock_delete(x) pthread_mutex_destroy(&(x))

#endif

[wgarvin]

GCC with -O2 setting optimizes very aggressively. In most cases where it makes a potentially 'unsafe' assumption it gives at least a warning. I had to rewrite some code pieces due to pointer-aliasing but that was pretty straightforward.

Usually its the programmer who makes the potentially unsafe assumptions!

Anyone who's never seen it before might find this set of slides interesting: Dangerous Optimizations and the Loss of Causality

It has examples of various undefined or implementation-defined behaviours such as signed int overflow, pointer math overflow, and aliasing violations. Programmers used to do these things all the time and get away with it, but compilers are allowed to optimize away things with undefined results, and so some of these common tricks can have surprising results with an aggressive compiler.

[mcostalba]

The biggest headache was with the following code:

I remember this code chunk very very well !!!

It took us a terrible, painful and several days long debugging session to find it !

As is reported on the comment

Code: Select all

// Extracting the _signed_ lower and upper 16 bits it not so trivial
// because according to the standard a simple cast to short is
// implementation defined and so is a right shift of a signed integer.

It means that gcc can and do aggresively optimize using also the smallest trick and obscure corner case that the C++ standard allows, and this potentially breaks a lot of implementations based on "common practices" or "well known" tricks.

The problem anyhow is in the code that is not fully 100% standard compliant, not in gcc.

The best approach is to _always_ avoid tricks, like sneak in two values in a single variable. When, as in this case, we really want to be "cool" then we also must be very careful about what we are doing.

[jshriver]

This was an enjoyable read. Sorry I didn't realise you were the author of Critter. I've enjoyed playing it.

[hgm]

Wouldn't it be better to simply define it as a struct/union?

Code: Select all

typedef union {
  int i;
  struct {
    short int eg_value;
    short int mg_value;
  } s;
} ScorePair;

Then you can write score.s.eg_value in stead of eg_value(score), and you can still access it as an int through score.i. Or do you rely on the order in which the two halves are stored in the int?

[jshriver]

Will you also be releasing the linux version anytime soon been anxious to try 0.5 especially if there is a linux port. Been using the win32 under wine.

-Josh

[bob]

I just want to share my experience with porting from win/msvc to linux/gcc.

GCC with -O2 setting optimizes very aggressively. In most cases where it makes a potentially 'unsafe' assumption it gives at least a warning. I had to rewrite some code pieces due to pointer-aliasing but that was pretty straightforward.
The biggest headache was with the following code:

Code: Select all

inline Score eg_value(ScorePair s) {
  return Score(short(s & 0xffff));
}

inline Score mg_value(ScorePair s) {
  return Score((int(s) + 0x8000) >> 16);
}

First, are you _really_ storing two signed values in 32 bits? Why not use excess-N notation so that you don't have to worry about the sign extension stuff.

For example, assuming your values are -32768 < N < +32767 (normal signed short range), and given two values s1 and s2, why not:

uint32_t merge = (s1 + 32768) << 16 + (s2 + 32768};

To get 'em back:

s1 = (merge >> 16) - 32768;
s2 = (merge & 0xffff) - 32768;

Now you don't have to worry about sign extension at all. (parens added for clarity).

What is the type "Score"? short or int?

One of the two returns has to do something since one returns an int value, one returns a short value.

That is not really something that I would complain about with respect to gcc, it is a really bad coding style in the first place... Not all machines have a sar instruction (x86 does). The C standards group left a bit too much "unspecified".

The way I did it above, you don't shift signed values, so that shl,shr are used and the sign bit is just a part of the value, not a sign at all...

MSVC (and GCC with -O1) does understand this, and correctly sign-extends by emmitting movsx/cwd/cdq instructions depending on context. However, GCC with -O2 completely optimized away eg_value(), and did some very weird things with mg_value().

In desperation, I took a look at StockFish's value.h and indeed found a solution therein (SF guys are much better at C/C++ stuff than me).
Now this code works correctly:

Code: Select all

#ifdef __GNUC__

inline Score eg_value(ScorePair s) {
  return Score((int)(unsigned(s) & 0x7fffu) - (int)(unsigned(s) & 0x8000u));
}

inline Score mg_value(ScorePair s) {
  return Score(((int(s) + 0x8000) & ~0xffff) / 0x10000);
}

#else

inline Score eg_value(ScorePair s) {
  return Score(short(s & 0xffff));
}

inline Score mg_value(ScorePair s) {
  return Score((int(s) + 0x8000) >> 16);
}

#endif

Big thanks to the StockFish team! I owe them some beer

Contrary to my fears, the threading, locking and signaling stuff wasn't very hard to port. In case someone finds it useful here is my smp.h:

Code: Select all

#pragma once

#ifdef _WIN32

#include <windows.h>

class Event {
  HANDLE hEvent;
public:
  inline Event() {};

  inline void init() { 
    hEvent = CreateEvent(NULL, false, false, NULL); 
  }

  inline void destroy() { 
    CloseHandle(hEvent); 
  }

  inline void signal() { 
    SetEvent(hEvent); 
  }

  inline void wait_for() { 
    WaitForSingleObject(hEvent, INFINITE); 
  }

  inline bool wait_for(int timeout) {
    return (WaitForSingleObject(hEvent, timeout) == WAIT_TIMEOUT);
  }

};

#define ThreadProcResult DWORD WINAPI
typedef LPVOID ThreadProcParam;
typedef ThreadProcResult ThreadProc(ThreadProcParam);

inline void start_thread(ThreadProc proc, ThreadProcParam param) {
  DWORD tid;
  CloseHandle(CreateThread(NULL, 0, proc, param, 0, &tid));
}

typedef CRITICAL_SECTION Lock;

#define lock_init(lock) InitializeCriticalSection(&(lock))
#define lock_acquire(lock) EnterCriticalSection(&(lock))
#define lock_release(lock) LeaveCriticalSection(&(lock))
#define lock_delete(lock) DeleteCriticalSection(&(lock))

#else

#include <pthread.h>
#include <errno.h>

class Event {
  pthread_cond_t cond;
  pthread_mutex_t mutex;
  volatile bool signaled;
public:
  inline Event() {};

  inline void init() {
    pthread_cond_init(&cond, NULL);
    pthread_mutex_init(&mutex, NULL);
    signaled = false;
  }

  inline void destroy() {
    pthread_cond_destroy(&cond);
    pthread_mutex_destroy(&mutex);
  }

  inline void signal() {
    pthread_mutex_lock(&mutex);
    pthread_cond_broadcast(&cond);
    signaled = true;
    pthread_mutex_unlock(&mutex);
  }

  inline void wait_for() {
    pthread_mutex_lock(&mutex);
    if (!signaled)
      pthread_cond_wait(&cond, &mutex);
    signaled = false;
    pthread_mutex_unlock(&mutex);
  }

  inline bool wait_for(int timeout) {
    pthread_mutex_lock(&mutex);
    int res = 0;
    if (!signaled) {
      struct timespec abstime;
      abstime.tv_sec  = timeout / 1000000;
      abstime.tv_nsec = timeout % 1000000;
      res = pthread_cond_timedwait(&cond, &mutex, &abstime);
    }
    signaled = false;
    pthread_mutex_unlock(&mutex);
    return (res == ETIMEDOUT);
  }
};

#define ThreadProcResult void *
typedef void * ThreadProcParam;
typedef ThreadProcResult ThreadProc(ThreadProcParam);

inline void start_thread(ThreadProc proc, ThreadProcParam param) {
  pthread_t pthread[1];
  pthread_attr_t attrib;
  pthread_attr_init(&attrib);
  pthread_create(pthread, &attrib, proc, param);
}

typedef pthread_mutex_t Lock;

#define lock_init(x) pthread_mutex_init(&(x), NULL)
#define lock_acquire(x) pthread_mutex_lock(&(x))
#define lock_release(x) pthread_mutex_unlock(&(x))
#define lock_delete(x) pthread_mutex_destroy(&(x))

#endif

[mcostalba]

Wouldn't it be better to simply define it as a struct/union?

No. The trick here is that you can sum/subtract two values in a single operation:

Code: Select all

s = s1 + s2;

Instead of writing

Code: Select all

s.eg_value = s1.eg_value + s2.eg_value;
s.mg_value = s1.mg_value+ s2.mg_value;

[Evert]

Wouldn't it be better to simply define it as a struct/union?

No. The trick here is that you can sum/subtract two values in a single operation:

Code: Select all

s = s1 + s2;

You can do that using the i field in the union.
You don't know whether the middle-game or the end-game value is the high or the low bit, but you don't need to know that either, since you'd use the struct inside the union to acces them when you need to.

[UncombedCoconut]

Wouldn't it be better to simply define it as a struct/union?

Code: Select all

typedef union {
  int i;
  struct {
    short int eg_value;
    short int mg_value;
  } s;
} ScorePair;

Then you can write score.s.eg_value in stead of eg_value(score), and you can still access it as an int through score.i. Or do you rely on the order in which the two halves are stored in the int?

I once tried replacing Stockfish's Score type with a struct that used two 16-bit fields, and got identical benchmark results. The result surprised me at the time. (I had a pre-conceived notion that the compiler would generate inefficient code for loading and storing bitfields.)