compiler dependent scores!

[jswaff]

Every good compiler builds a dependency graph before it starts to muck around with loops. And then they ask the question "for all loop iterations i, does any single iteration i[n] depend on the results of any previous iteration completing first?" If the answer is no, then it does not matter whether the loop is executed first-to-last, last-to-first, or in completely random order.

The dependency graph will detect inter-loop dependencies and prevent this from occurring if it would cause a problem.

So in your opinion, is this a compiler bug?

--
James

[Gerd Isenberg]

Code: Select all

        for (int c=0;c<64;c++) {
                to_boundary[c]=0;
                for (int t=1;t<8;t++) {
//                      if (c==E4) printf("considering: %d\n",t);
                        if (c-(t*8)<0) break;
                        to_boundary[c]|=bm_mask[c-(t*8)];
                }
        }

Just another try to avoid redundant repeat/break condition of the inner loop as a source of possible compiler confusion...

Code: Select all

   
   for (int c=0; c<64; c++) {
      to_boundary[c]=0;
      for (int t=8; t <= c; t += 8) {
         // if (c==E4) printf("considering: %d\n",t>>3);
         to_boundary[c] |= bm_mask[c-t];
      }
   }

I regocnized your big-endian rank mapping, thus the initialization code I posted before has to be adapted, since it relies on a1 == 0.

Your code works. I get a couple of remarks from the compiler about the partial loop being vectorized, but it works correctly.

That said, despite the fact that you don't like the break in the inner loop, it IS legal as far as I know. And it's more obvious (to me) what's going on.


--
James

Nothing against the break, but I prefer code without redundant repeat conditions - I like a < b better than a - b < 0, and prefer explicit loop-hoisting - or, as mentioned, to avoid loops to synthesize those bitboards from single bits at all

You may change the redundant t < 8 to 1, "true" or to t > 0 or leave it blank. Does this code work? I guess yes. This is a compiler bug - the redundant t < 8 condition wrongly guides the compiler to change the order of execution, because it somehow misses the order-relevance of the "obfuscated" break-condition.

Cheers,
Gerd

[Carey]

A bit sloppy?? It is a "best practice" to minimize the scope of variables.
It improves readability, and more importantly, limits visibility of the variable. True, in C, you *have* to declare local variables at the head of the block, but that is definitely a habit worth breaking.

Actually, you don't have to do that in C.

The 1999 C standard added a number of useful features, such as specified size data types (uint64_t, for example) along with // comments, complex numbers, and the ability to specify local variables where ever you need to, including within a for (...) statement.

Quite a few interesting additions over C89.

The problem is, even after 9 years, many compilers *still* don't support the current C version.

C just isn't "sexy" any more, so vendors just don't care as much as they do about C++.


I'll admit that many of the things in C99 aren't absolute requirements. But some of them sure are nice. Things like uint64_t for example. And knowing you can legally do // comments. And even localized local vars like this discussion is about.


Personally, I think that if your compiler can't handle a 9 year old C standard (or at least attempts to support many of the simpler additions), then it's time to find a compiler that does.

If you are forced to stay with C89, a 19 year old standard, then you might as well go back one more year and stay with K&R C...

(Or use Small-C.... That was kind of fun to program in.)

[Alessandro Scotti]

I agree about the FOR loop. I assumed the loop was iterated sequentially t=1..7, so I guess that's the root problem.

James, do not blame on yourself the compiler bugs. The assumption that the loop MUST be executed sequentially is entirely correct, it's what structured programming is all about in fact.
Here the compiler thinks it knows better and fails. Happened to me once and I never used that compiler anymore, have enough of my own bugs thank you!

[jswaff]

Code: Select all

        for (int c=0;c<64;c++) {
                to_boundary[c]=0;
                for (int t=1;t<8;t++) {
//                      if (c==E4) printf("considering: %d\n",t);
                        if (c-(t*8)<0) break;
                        to_boundary[c]|=bm_mask[c-(t*8)];
                }
        }

Just another try to avoid redundant repeat/break condition of the inner loop as a source of possible compiler confusion...

Code: Select all

   
   for (int c=0; c<64; c++) {
      to_boundary[c]=0;
      for (int t=8; t <= c; t += 8) {
         // if (c==E4) printf("considering: %d\n",t>>3);
         to_boundary[c] |= bm_mask[c-t];
      }
   }

I regocnized your big-endian rank mapping, thus the initialization code I posted before has to be adapted, since it relies on a1 == 0.

Your code works. I get a couple of remarks from the compiler about the partial loop being vectorized, but it works correctly.

That said, despite the fact that you don't like the break in the inner loop, it IS legal as far as I know. And it's more obvious (to me) what's going on.


--
James

Nothing against the break, but I prefer code without redundant repeat conditions - I like a < b better than a - b < 0, and prefer explicit loop-hoisting - or, as mentioned, to avoid loops to synthesize those bitboards from single bits at all

You may change the redundant t < 8 to 1, "true" or to t > 0 or leave it blank. Does this code work? I guess yes. This is a compiler bug - the redundant t < 8 condition wrongly guides the compiler to change the order of execution, because it somehow misses the order-relevance of the "obfuscated" break-condition.

Cheers,
Gerd

You're right, removing the redundant t<8 condition does the trick. So this IS a compiler bug.

I appreciate your comments. You are a world class bit twiddler for sure. One of these days I'll get to doing some of those types of optimizations, but in general I think it's more important to have code you understand. It's obvious to me (having written it) that "c-(t*8)<0" means "if we move up another rank and are still 'on the board'." Yes, "c<(t*8)" is logically equivalent, but I'd have to think about what it means just a little longer. Besides, the compiler shouldn't have any trouble optimizing that one.

Re: constructing the bitmap by synthesizing from single bits; yes, less efficient. But, this is initialization code.

--
James

[jswaff]

I agree about the FOR loop. I assumed the loop was iterated sequentially t=1..7, so I guess that's the root problem.

James, do not blame on yourself the compiler bugs. The assumption that the loop MUST be executed sequentially is entirely correct, it's what structured programming is all about in fact.
Here the compiler thinks it knows better and fails. Happened to me once and I never used that compiler anymore, have enough of my own bugs thank you!

Well, I started out thinking this might be a compiler bug, convinced myself it was, then started to question if my assumption about the sequential ordering of the loop body was correct. Glad to hear it was.

--
James

[Alessandro Scotti]

Re: constructing the bitmap by synthesizing from single bits; yes, less efficient. But, this is initialization code.

I have similar initialization stuff in Kiwi. One "abstraction" that I found useful is to see the bitboard as a "bitmap". Then I have functions to draw "pixels", "lines" and "rectangles". With that, writing initialization code was almost fun!

[jswaff]

I didn't have time to completely respond earlier. But here is a question:

Here are two pieces of code that do the _identically_ same computations:

Code: Select all

int EvaluateKingsFile(TREE * RESTRICT tree, int whichfile, int side)
{
  register int defects = 0, file;
  register int enemy = Flip(side);
  static const int a2a7[2] = { A7, A2 }; 
  static const int a3a6[2] = { A6, A3 };
    
  for (file = whichfile - 1; file <= whichfile + 1; file++) {
    if (!(file_mask[file] & tree->all_pawns))
      defects += open_file[file];
    else {
      if (!(file_mask[file] & Pawns(enemy)))
        defects += half_open_file[file] / 2;
      else
        defects +=
            pawn_defects[side][Rank(Advanced(enemy,
                    file_mask[file] & Pawns(enemy)))];
      if (!(file_mask[file] & Pawns(side)))
        defects += half_open_file[file];
      else {
        if (!(Pawns(side) & SetMask(a2a7[side] + file))) {
          defects++;
          if (!(Pawns(side) & SetMask(a3a6[side] + file)))
            defects++;
        }
      }
    }
  }
  return (defects);
}

and then

Code: Select all

int EvaluateKingsFile(TREE * RESTRICT tree, int whichfile, int side) { register int defects = 0, file; register int enemy = Flip(side); static const int a2a7[2] = { A7, A2 }; static const int a3a6[2] = { A6, A3 }; for (file = whichfile - 1; file <= whichfile + 1; file++) { if (!(file_mask[file] & tree->all_pawns)) defects += open_file[file]; else { if (!(file_mask[file] & Pawns(enemy))) defects += half_open_file[file] / 2; else defects += pawn_defects[side][Rank(Advanced(enemy, file_mask[file] & Pawns(enemy)))]; if (!(file_mask[file] & Pawns(side))) defects += half_open_file[file]; else { if (!(Pawns(side) & SetMask(a2a7[side] + file))) { defects++; if (!(Pawns(side) & SetMask(a3a6[side] + file))) defects++; } } } } return (defects); }

Which one is easier to read? that is what I don't like about the

for (int i=0; i<n; i++)

syntax. that "int i" is buried inside a common statement and it is easy to overlook the fact that i is local to the loop only, and if you modify i inside the loop, or break out of the loop, the value of i that is left is not the one from the loop counter, it is the i that has greater scope.

It's generally a bad idea to try to use a loop counter outside the loop anyway. IIRC the value of the loop counter when you exit the loop is undefined.

Some compilers will complain, some will not, when that happens, just like some complain when a formal parameter shadows a global variable. I think that it makes it easier to explicitly declare where it is visible, rather than using declarations that are difficult to pick out. That was my point... anything to eliminate bugs is good. That's why we don't cram everything onto one line...

Well Bob, I agree that anything to eliminate bugs is good. But that's kind of the point of minimizing the scope of variables. To extend your reasoning - why not declare every variable in the whole program global? Then you'd never have to worry about a variable local to some function shadowing a global.

Declaring a variable outside the block is is first used in is just asking for bugs IMHO; you could accidentally use it before or after the block you intend to use it in.

You don't have to take my word for it.. I didn't declare it a "best practice."

--
James

[bob]

I didn't have time to completely respond earlier. But here is a question:

Here are two pieces of code that do the _identically_ same computations:

Code: Select all

int EvaluateKingsFile(TREE * RESTRICT tree, int whichfile, int side)
{
  register int defects = 0, file;
  register int enemy = Flip(side);
  static const int a2a7[2] = { A7, A2 }; 
  static const int a3a6[2] = { A6, A3 };
    
  for (file = whichfile - 1; file <= whichfile + 1; file++) {
    if (!(file_mask[file] & tree->all_pawns))
      defects += open_file[file];
    else {
      if (!(file_mask[file] & Pawns(enemy)))
        defects += half_open_file[file] / 2;
      else
        defects +=
            pawn_defects[side][Rank(Advanced(enemy,
                    file_mask[file] & Pawns(enemy)))];
      if (!(file_mask[file] & Pawns(side)))
        defects += half_open_file[file];
      else {
        if (!(Pawns(side) & SetMask(a2a7[side] + file))) {
          defects++;
          if (!(Pawns(side) & SetMask(a3a6[side] + file)))
            defects++;
        }
      }
    }
  }
  return (defects);
}

and then

Code: Select all

int EvaluateKingsFile(TREE * RESTRICT tree, int whichfile, int side) { register int defects = 0, file; register int enemy = Flip(side); static const int a2a7[2] = { A7, A2 }; static const int a3a6[2] = { A6, A3 }; for (file = whichfile - 1; file <= whichfile + 1; file++) { if (!(file_mask[file] & tree->all_pawns)) defects += open_file[file]; else { if (!(file_mask[file] & Pawns(enemy))) defects += half_open_file[file] / 2; else defects += pawn_defects[side][Rank(Advanced(enemy, file_mask[file] & Pawns(enemy)))]; if (!(file_mask[file] & Pawns(side))) defects += half_open_file[file]; else { if (!(Pawns(side) & SetMask(a2a7[side] + file))) { defects++; if (!(Pawns(side) & SetMask(a3a6[side] + file))) defects++; } } } } return (defects); }

Which one is easier to read? that is what I don't like about the

for (int i=0; i<n; i++)

syntax. that "int i" is buried inside a common statement and it is easy to overlook the fact that i is local to the loop only, and if you modify i inside the loop, or break out of the loop, the value of i that is left is not the one from the loop counter, it is the i that has greater scope.

It's generally a bad idea to try to use a loop counter outside the loop anyway. IIRC the value of the loop counter when you exit the loop is undefined.

Heavens no, and thank goodness the days of FORTRAN are gone. Loops like this exist in every program I have looked at:

For (i=0; i<x ;i++)
if (a < 0) break;

And then use i which points to the first element of a that is < 0...

Some compilers will complain, some will not, when that happens, just like some complain when a formal parameter shadows a global variable. I think that it makes it easier to explicitly declare where it is visible, rather than using declarations that are difficult to pick out. That was my point... anything to eliminate bugs is good. That's why we don't cram everything onto one line...

Well Bob, I agree that anything to eliminate bugs is good. But that's kind of the point of minimizing the scope of variables. To extend your reasoning - why not declare every variable in the whole program global? Then you'd never have to worry about a variable local to some function shadowing a global.

Again, it is not the scope I am complaining about. This is fine:

for (i=i ....) {
int j, k;


}

The problem I had with your code was that this:

for (int i=0; ...) {


}

hides that int declaration pretty effectively, particularly if the rest of the for is more wordy or the termination test is more complex... I didn't like the "int" being buried in a place it is normally not found, because it makes it too easy to overlook.

So this isn't about global vs local, or even about scope. It is just about how something is declared to make it obvious what is going on...

Declaring a variable outside the block is is first used in is just asking for bugs IMHO; you could accidentally use it before or after the block you intend to use it in.

You don't have to take my word for it.. I didn't declare it a "best practice."

--
James

[bob]

I agree about the FOR loop. I assumed the loop was iterated sequentially t=1..7, so I guess that's the root problem.

James, do not blame on yourself the compiler bugs. The assumption that the loop MUST be executed sequentially is entirely correct, it's what structured programming is all about in fact.
Here the compiler thinks it knows better and fails. Happened to me once and I never used that compiler anymore, have enough of my own bugs thank you!

Actually this is wrong. There is no explicit directions about order of loop execution, as you can't even guarantee that if you write in assembly language, because the processor also adds out-of-order execution to the mix. All that is required is that the data dependencies must be respected, not that the order must be maintained...