Crafty c questions

Discussion of chess software programming and technical issues.

Moderators: hgm, Rebel, chrisw

jwes
Posts: 778
Joined: Sat Jul 01, 2006 7:11 am

Crafty c questions

Post by jwes »

I am trying to compile crafty 25.0 with MSVC 2013 and am running into some errors.

1. This line appears in init.c

Code: Select all

memset((void *) hash_table + node * mem_per_node, 0, mem_per_node);
Should this be?

Code: Select all

memset((char*) hash_table + node * mem_per_node, 0, mem_per_node);
Or perhaps?

Code: Select all

 memset((void *) ((char *)hash_table + node * mem_per_node), 0, mem_per_node);
2. This function is used if INLINEASM is not defined and does not compile:

Code: Select all

int PopCnt(uint64_t arg1) {
  int c;

  for (c = 0; x; c++)
    x &= x - 1;
  return c;
}
3. Bob, do you ever compile and run with DEBUG defined? I get runtime errors in 24.1 if I try.

4. Will it still compile in 32 bit mode?
Joost Buijs
Posts: 1563
Joined: Thu Jul 16, 2009 10:47 am
Location: Almere, The Netherlands

Re: Crafty c questions

Post by Joost Buijs »

jwes wrote:I am trying to compile crafty 25.0 with MSVC 2013 and am running into some errors.

1. This line appears in init.c

Code: Select all

memset((void *) hash_table + node * mem_per_node, 0, mem_per_node);
This looks like a bug.
I guess you have to add some parentheses like:

Code: Select all

memset((void *) (hash_table + node * mem_per_node), 0, mem_per_node);
jwes wrote: 2. This function is used if INLINEASM is not defined and does not compile:

Code: Select all

int PopCnt(uint64_t arg1) {
  int c;

  for (c = 0; x; c++)
    x &= x - 1;
  return c;
}
You have to replace arg1 by x, then it will compile.
mar
Posts: 2554
Joined: Fri Nov 26, 2010 2:00 pm
Location: Czech Republic
Full name: Martin Sedlak

Re: Crafty c questions

Post by mar »

Well, it seems that Crafty is indeed full of typos:

Code: Select all

  AlignedMalloc((void *) ((void *) &hash_table), 64,
      sizeof(HASH_ENTRY) * hash_table_size);
The first cast should've been to void ** (btw such code wouldn't even compile in C++, but it does in C) - actually why two casts is beyond my imagination.
There are other subtle issues like leaking memory in errorneous situations (I think book code or something like that).
EDIT: speaking of C only, you actually don't have to cast at all
bob
Posts: 20943
Joined: Mon Feb 27, 2006 7:30 pm
Location: Birmingham, AL

Re: Crafty c questions

Post by bob »

mar wrote:Well, it seems that Crafty is indeed full of typos:

Code: Select all

  AlignedMalloc((void *) ((void *) &hash_table), 64,
      sizeof(HASH_ENTRY) * hash_table_size);
The first cast should've been to void ** (btw such code wouldn't even compile in C++, but it does in C) - actually why two casts is beyond my imagination.
There are other subtle issues like leaking memory in errorneous situations (I think book code or something like that).
EDIT: speaking of C only, you actually don't have to cast at all
(a) you have to cast to avoid warnings. That's the only purpose for the ones you mention. These were sent to me by someone that was trying to eliminate some warnings on their compiler. I have no idea why a recast would cause problems on C++. There's nothing wrong with

(void *) ((void * x)) other than redundancy

(b) I am not aware of any memory leaks, which would cause the program to grow without constraint.
Joost Buijs
Posts: 1563
Joined: Thu Jul 16, 2009 10:47 am
Location: Almere, The Netherlands

Re: Crafty c questions

Post by Joost Buijs »

mar wrote:Well, it seems that Crafty is indeed full of typos:

Code: Select all

  AlignedMalloc((void *) ((void *) &hash_table), 64,
      sizeof(HASH_ENTRY) * hash_table_size);
The first cast should've been to void ** (btw such code wouldn't even compile in C++, but it does in C) - actually why two casts is beyond my imagination.
There are other subtle issues like leaking memory in errorneous situations (I think book code or something like that).
EDIT: speaking of C only, you actually don't have to cast at all
One of the casts to (void *) is redundant, it actually does nothing at all.
The last time I looked at the Crafty source is at least 16 years ago, I have no idea what hash_table is, when I look at this code it does not seem to be a pointer.

I have no idea whether you have to cast to (void *) under plain C or not. According to K&R it is not necessary, it also depends upon the compiler I suppose.
bob
Posts: 20943
Joined: Mon Feb 27, 2006 7:30 pm
Location: Birmingham, AL

Re: Crafty c questions

Post by bob »

Joost Buijs wrote:
mar wrote:Well, it seems that Crafty is indeed full of typos:

Code: Select all

  AlignedMalloc((void *) ((void *) &hash_table), 64,
      sizeof(HASH_ENTRY) * hash_table_size);
The first cast should've been to void ** (btw such code wouldn't even compile in C++, but it does in C) - actually why two casts is beyond my imagination.
There are other subtle issues like leaking memory in errorneous situations (I think book code or something like that).
EDIT: speaking of C only, you actually don't have to cast at all
One of the casts to (void *) is redundant, it actually does nothing at all.
The last time I looked at the Crafty source is at least 16 years ago, I have no idea what hash_table is, when I look at this code it does not seem to be a pointer.

I have no idea whether you have to cast to (void *) under plain C or not. According to K&R it is not necessary, it also depends upon the compiler I suppose.

Here's the answer to your question:

for memset, which is used to clear the stuff:

SYNOPSIS
#include <string.h>

void *
memset(void *b, int c, size_t len);

For malloc() which is used to allocate memory:

SYNOPSIS
#include <stdlib.h>

void *
malloc(size_t size);


(void *) is the correct definition for passing a malloc-type pointer to system calls such as memset and such. The intent with all of this was to eliminate 100% of the compiler warnings.
bob
Posts: 20943
Joined: Mon Feb 27, 2006 7:30 pm
Location: Birmingham, AL

Re: Crafty c questions

Post by bob »

jwes wrote:I am trying to compile crafty 25.0 with MSVC 2013 and am running into some errors.

1. This line appears in init.c

Code: Select all

memset&#40;&#40;void *) hash_table + node * mem_per_node, 0, mem_per_node&#41;;
Should this be?

Code: Select all

memset&#40;&#40;char*) hash_table + node * mem_per_node, 0, mem_per_node&#41;;
Or perhaps?

Code: Select all

 memset&#40;&#40;void *) (&#40;char *&#41;hash_table + node * mem_per_node&#41;, 0, mem_per_node&#41;;
2. This function is used if INLINEASM is not defined and does not compile:

Code: Select all

int PopCnt&#40;uint64_t arg1&#41; &#123;
  int c;

  for &#40;c = 0; x; c++)
    x &= x - 1;
  return c;
&#125;
3. Bob, do you ever compile and run with DEBUG defined? I get runtime errors in 24.1 if I try.

4. Will it still compile in 32 bit mode?
It should compile as 32 bit executable assuming the compiler will accept uint64_t which gcc should.

I regularly run with -DDEBUG and it is working right now in fact. Can you post some output showing what is going wrong???

Here are two runs, first with -DDEBUG, second without. Only difference is the NPS.

23-> 25.27 -6.25 1. ... Rxb2 2. Rxb2 c3 3. Rb6+ Ke7 4. Kf2
c2 5. Rc6 d2 6. Rxc2 d1=Q 7. Rc5 Kd6
8. Re5 Qd3 9. Kf3 Qxa3 10. Rxf5 Qb2
11. Rf7 Qxh2 12. Ra7 Qc2 13. Rg7
time=25.27(100%) nodes=26431424(26.4M) fh1=91% pred=0 nps=1.0M
chk=1.1M qchk=1.4M fp=4.6M mcp=3.5M 50move=1
LMReductions: 1/786.8K 2/536.0K 3/282.9K 4/37.6K 5/351
null-move (R): 3/199.4K 4/82.0K 5/2.6K 6/113


and

23-> 3.77/21.60 -6.25 1. ... Rxb2 2. Rxb2 c3 3. Rb6+ Ke7 4. Kf2
c2 5. Rc6 d2 6. Rxc2 d1=Q 7. Rc5 Kd6
8. Re5 Qd3 9. Kf3 Qxa3 10. Rxf5 Qb2
11. Rf7 Qxh2 12. Ra7 Qc2 13. Rg7
time=3.77(100%) nodes=26431424(26.4M) fh1=91% pred=0 nps=7.0M
chk=1.1M qchk=1.4M fp=4.6M mcp=3.5M 50move=1
LMReductions: 1/786.8K 2/536.0K 3/282.9K 4/37.6K 5/351
null-move (R): 3/199.4K 4/82.0K 5/2.6K 6/113
Black(1): Rxb2

So about 7x slower. I use this regularly as it is a good sanity test on the code.

The double (void *) declarations should not cause any issues. IE

(int) ((int) x)

is perfectly acceptable C code. Extra typing. Where those double void * recasts came from I don't remember. I've removed them and will see who complains when 25.1 is released... The only problem is that the various hash tables are NOT pointers to void, they are pointers to structs, and not re-casting them to (void *) produces warnings...
bob
Posts: 20943
Joined: Mon Feb 27, 2006 7:30 pm
Location: Birmingham, AL

Re: Crafty c questions

Post by bob »

Joost Buijs wrote:
jwes wrote:I am trying to compile crafty 25.0 with MSVC 2013 and am running into some errors.

1. This line appears in init.c

Code: Select all

memset&#40;&#40;void *) hash_table + node * mem_per_node, 0, mem_per_node&#41;;
This looks like a bug.
I guess you have to add some parentheses like:

Code: Select all

memset&#40;&#40;void *) &#40;hash_table + node * mem_per_node&#41;, 0, mem_per_node&#41;;
jwes wrote: 2. This function is used if INLINEASM is not defined and does not compile:

Code: Select all

int PopCnt&#40;uint64_t arg1&#41; &#123;
  int c;

  for &#40;c = 0; x; c++)
    x &= x - 1;
  return c;
&#125;
You have to replace arg1 by x, then it will compile.
The parens will break the above.

If you have pointer + x, then the compiler will produce code that REALLY computes "pointer + sizeof(x)" That is not what is intended above. (void *) assumes the size of an object is 1, which makes this work correctly. The parens will produce seg faults because the pointers will go way out of bounds.
bob
Posts: 20943
Joined: Mon Feb 27, 2006 7:30 pm
Location: Birmingham, AL

Re: Crafty c questions

Post by bob »

jwes wrote:I am trying to compile crafty 25.0 with MSVC 2013 and am running into some errors.

1. This line appears in init.c

Code: Select all

memset&#40;&#40;void *) hash_table + node * mem_per_node, 0, mem_per_node&#41;;
Should this be?

Code: Select all

memset&#40;&#40;char*) hash_table + node * mem_per_node, 0, mem_per_node&#41;;
Or perhaps?

Code: Select all

 memset&#40;&#40;void *) (&#40;char *&#41;hash_table + node * mem_per_node&#41;, 0, mem_per_node&#41;;
2. This function is used if INLINEASM is not defined and does not compile:

Code: Select all

int PopCnt&#40;uint64_t arg1&#41; &#123;
  int c;

  for &#40;c = 0; x; c++)
    x &= x - 1;
  return c;
&#125;
3. Bob, do you ever compile and run with DEBUG defined? I get runtime errors in 24.1 if I try.

4. Will it still compile in 32 bit mode?
I just noticed you referenced 24.1 for the -DDEBUG question. I just tried 24.1 and it worked for me with -DDEBUG, also. If you can post (a) the output where it fails and (b) the starting position you used, I can test.
Joost Buijs
Posts: 1563
Joined: Thu Jul 16, 2009 10:47 am
Location: Almere, The Netherlands

Re: Crafty c questions

Post by Joost Buijs »

bob wrote: If you have pointer + x, then the compiler will produce code that REALLY computes "pointer + sizeof(x)
If you have 'pointer + x' you actually get the pointer + x * the size of the pointers object type and that is something completely different than 'pointer + sizeof(x)'.

A void pointer cannot be dereferenced, it has no associated object size attached to it. Maybe it is the difference between C and C++, the code we are taking about will not be accepted by a C++ compiler.

Maybe this would work:

Code: Select all

memset&#40;&#40;void *) (&#40;char *&#41;hash_table + node * mem_per_node&#41;, 0, mem_per_node&#41;;