quick question about compiling Stockfish with icc

Discussion of chess software programming and technical issues.

Moderator: Ras

bob
Posts: 20943
Joined: Mon Feb 27, 2006 7:30 pm
Location: Birmingham, AL

Re: quick question about compiling Stockfish with icc

Post by bob »

mcostalba wrote:
Don wrote:
bob wrote:
Don wrote:Dann or anyone,

I'm compiling with more warning enabled now. Now I get warning errors with bitfields in structures. It's really annoying because there seems to be no solution that I have found to prevent the warning generated going from a smaller fields to a 32 bit field.

Have you tried the obvious re-cast?
I stated the problem incorrectly, I'm trying to assign a smaller field from a larger integer type, otherwise I'm sure a cast would work fine.

How do you cast a 32 bit value to an 18 bit value so that the compiler will let it work? There is no syntax for doing that such as "(int:18) x"
Ok, sorry, but I have enabled the -pedantic flag also on myself now ;-)

Here we go, a little bit of annoying coding best practice lesson: shutting up a warning with a cast is normally the "The Wrong Thing To Do" (tm).

It means you have not clear what a warning is: It is not an error, it is something that the campiler wants "warn" you about. Look, this _could_ be dangerous. If you simply shut up the warning with a cast of course you do _not_ fix the source of danger that is to assign a 32 bit value to a smaller one, you simply say to the compiler: "no, I am not interested you warn me about this", but the source of warning is still alive and kicking. And if you change something in the behaviour of your code, that source of warning could _silently_ become a source of bugs.

So the bottom line is that using casts to shut up the compiler is generally a very bad idea and will lead to _real_ bugs.
There is "theory" and there is "reality". In theory, you are 100% correct. But in reality, if you get 100's of warnings, you will miss the _new_ warning from code you just added, that does represent a real bug. You have three choices...

(1) compile without all the warning options. Then you won't get tons of warnings, but you won't get the occasional important one you just caused, resulting in unnecessary debugging.

(2) ignore all the warnings, which is just as bad because you also ignore the important ones (that are new).

(3) fix them so that the compiler is happy. One could assume that if you do a re-cast, you know what you are doing. For example, in tcp/ip, I prefer "sockaddr_in" rather than "sockaddr" for the structure containing the ip address, port #, etc. Easier to use. But "connect()" wants a pointer to a structure of type sockaddr. I recast and everyone is happy, and silent.

If you use (3) you leave yourself open to problem caused by a warning that is not important today, but which might (as you said) be important tomorrow. But if you use options (1) or (2) you run even higher risks.
mcostalba
Posts: 2684
Joined: Sat Jun 14, 2008 9:17 pm

Re: quick question about compiling Stockfish with icc

Post by mcostalba »

bob wrote:
mcostalba wrote:
Don wrote:
bob wrote:
Don wrote:Dann or anyone,

I'm compiling with more warning enabled now. Now I get warning errors with bitfields in structures. It's really annoying because there seems to be no solution that I have found to prevent the warning generated going from a smaller fields to a 32 bit field.

Have you tried the obvious re-cast?
I stated the problem incorrectly, I'm trying to assign a smaller field from a larger integer type, otherwise I'm sure a cast would work fine.

How do you cast a 32 bit value to an 18 bit value so that the compiler will let it work? There is no syntax for doing that such as "(int:18) x"
Ok, sorry, but I have enabled the -pedantic flag also on myself now ;-)

Here we go, a little bit of annoying coding best practice lesson: shutting up a warning with a cast is normally the "The Wrong Thing To Do" (tm).

It means you have not clear what a warning is: It is not an error, it is something that the campiler wants "warn" you about. Look, this _could_ be dangerous. If you simply shut up the warning with a cast of course you do _not_ fix the source of danger that is to assign a 32 bit value to a smaller one, you simply say to the compiler: "no, I am not interested you warn me about this", but the source of warning is still alive and kicking. And if you change something in the behaviour of your code, that source of warning could _silently_ become a source of bugs.

So the bottom line is that using casts to shut up the compiler is generally a very bad idea and will lead to _real_ bugs.
There is "theory" and there is "reality". In theory, you are 100% correct. But in reality, if you get 100's of warnings, you will miss the _new_ warning from code you just added, that does represent a real bug. You have three choices...

(1) compile without all the warning options. Then you won't get tons of warnings, but you won't get the occasional important one you just caused, resulting in unnecessary debugging.

(2) ignore all the warnings, which is just as bad because you also ignore the important ones (that are new).

(3) fix them so that the compiler is happy. One could assume that if you do a re-cast, you know what you are doing. For example, in tcp/ip, I prefer "sockaddr_in" rather than "sockaddr" for the structure containing the ip address, port #, etc. Easier to use. But "connect()" wants a pointer to a structure of type sockaddr. I recast and everyone is happy, and silent.

If you use (3) you leave yourself open to problem caused by a warning that is not important today, but which might (as you said) be important tomorrow. But if you use options (1) or (2) you run even higher risks.
You miss option 4:

(4) Rewrite your code to avoid warnings in first instance even without using casts.

And no, you cannot convince me that cast are _necessary_ to silence hundreds of warnings, in that case you have to seriously think to a different choice of data structures layout for your program. You could have some warnings also in a properly written program, but if you have hundreds of them and the only way to "fix" them is to use cast then I think you have a bigger problem then the warnings with your code: you have a mess in your hands !! And a deep code redesign is urgently required.

BTW in SF there are almost no cast, I only in exceptional cases use them and I prefer some warnings (very few BTW) in pawns.cpp than to dumbly cast an int to a int16_t, so that every time I compile compiler makes me "remember" that in that part of code we could do better....and we have done because in the current dev version even that warnings have been removed...and without cast :-)
User avatar
michiguel
Posts: 6401
Joined: Thu Mar 09, 2006 8:30 pm
Location: Chicago, Illinois, USA

Re: quick question about compiling Stockfish with icc

Post by michiguel »

mcostalba wrote:
Don wrote:
bob wrote:
Don wrote:Dann or anyone,

I'm compiling with more warning enabled now. Now I get warning errors with bitfields in structures. It's really annoying because there seems to be no solution that I have found to prevent the warning generated going from a smaller fields to a 32 bit field.

Have you tried the obvious re-cast?
I stated the problem incorrectly, I'm trying to assign a smaller field from a larger integer type, otherwise I'm sure a cast would work fine.

How do you cast a 32 bit value to an 18 bit value so that the compiler will let it work? There is no syntax for doing that such as "(int:18) x"
Ok, sorry, but I have enabled the -pedantic flag also on myself now ;-)

Here we go, a little bit of annoying coding best practice lesson: shutting up a warning with a cast is normally the "The Wrong Thing To Do" (tm).

It means you have not clear what a warning is: It is not an error, it is something that the campiler wants "warn" you about. Look, this _could_ be dangerous. If you simply shut up the warning with a cast of course you do _not_ fix the source of danger that is to assign a 32 bit value to a smaller one, you simply say to the compiler: "no, I am not interested you warn me about this", but the source of warning is still alive and kicking. And if you change something in the behaviour of your code, that source of warning could _silently_ become a source of bugs.

So the bottom line is that using casts to shut up the compiler is generally a very bad idea and will lead to _real_ bugs.
No, it is a very good idea if the code reflect your intentions. Sometimes you really need to transfer data between variables of different sizes and there is no way you can get around it. For instance, in the interface that communicate two different data structures when you are packing information from int's to chars. To avoid a bug, you may use an assert in the interface.

Another example, there is a code that has been used for 10 years with no bugs detected. You decide to use that library but it comes with warnings. It is much safer to introduce one cast, than trying to modify the code and introduce a bug in a bug free code that you did not write, and most likely may not understand in its entirety. I needed to do that when I use compression libraries. I changed some code when I was 101% sure of a simple change, but most of the times, I used obvious casts.

There are many other examples.

Miguel
User avatar
Don
Posts: 5106
Joined: Tue Apr 29, 2008 4:27 pm

Re: quick question about compiling Stockfish with icc

Post by Don »

mcostalba wrote:
Don wrote:
bob wrote:
Don wrote:Dann or anyone,

I'm compiling with more warning enabled now. Now I get warning errors with bitfields in structures. It's really annoying because there seems to be no solution that I have found to prevent the warning generated going from a smaller fields to a 32 bit field.

Have you tried the obvious re-cast?
I stated the problem incorrectly, I'm trying to assign a smaller field from a larger integer type, otherwise I'm sure a cast would work fine.

How do you cast a 32 bit value to an 18 bit value so that the compiler will let it work? There is no syntax for doing that such as "(int:18) x"
Ok, sorry, but I have enabled the -pedantic flag also on myself now ;-)

Here we go, a little bit of annoying coding best practice lesson: shutting up a warning with a cast is normally the "The Wrong Thing To Do" (tm).

It means you have not clear what a warning is: It is not an error, it is something that the campiler wants "warn" you about. Look, this _could_ be dangerous. If you simply shut up the warning with a cast of course you do _not_ fix the source of danger that is to assign a 32 bit value to a smaller one, you simply say to the compiler: "no, I am not interested you warn me about this", but the source of warning is still alive and kicking. And if you change something in the behaviour of your code, that source of warning could _silently_ become a source of bugs.

So the bottom line is that using casts to shut up the compiler is generally a very bad idea and will lead to _real_ bugs.
A cast tells the compiler that you know what you want to do, it's not for shutting up the compiler.