Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Optimize an expression in endgame.cpp #1644

Closed

Conversation

protonspring
Copy link

This is non-functional. I believe using foward_file_bb here is fewer instructions.

GOOD) fewer instructions and probably more clear (debatable).

BAD) Possible that a lookup is slower than a few local operations, but the forward_file_bb table is probably used often enough that it is always cached.

STC
LLR: 2.96 (-2.94,2.94) [-3.00,1.00]
Total: 21004 W: 4263 L: 4141 D: 12600
http://tests.stockfishchess.org/tests/view/5b1cad830ebc5902ab9c6239

@protonspring
Copy link
Author

One thought. This code is only used for a specialized ending (KRKP). I guess it's possible that too many simplifications here could pass if the games don't use these endings much (it would be no different from master in this case). Is there any way to specifically test these kinds of endings to make sure there really is no elo loss?

@Rocky640
Copy link

Looks good to me, However, I would prefer
if (forward_file_bb(WHITE, wksq) & psq)

@ceebo
Copy link

ceebo commented Jun 10, 2018

The patch doesn't look correct to me. psq is relative to the strong side so you can't use it to test the squares of a Bitboard.

Many of the endgame functions are written in this style. I guess the reason is that most of the code had its origins in Glaurung 1.0 which was a mailbox engine (?)

@Rocky640
Copy link

@ceebo

I might be missing something, but my understanding is that
a) The following lines insures that everything is "normalized" for the strongSide changed to WHITE

  Square wksq = relative_square(strongSide, pos.square<KING>(strongSide));
  Square bksq = relative_square(strongSide, pos.square<KING>(weakSide));
  Square rsq  = relative_square(strongSide, pos.square<ROOK>(strongSide));
  Square psq  = relative_square(strongSide, pos.square<PAWN>(weakSide));

b) After above normalisation, the next line shows that master is expecting the weakside pawn to move towards the first rank

 Square queeningSq = make_square(file_of(psq), RANK_1);

c) Finally the single line change

if (forward_file_bb(WHITE, wksq) & psq) 

seems logically equivalent to this

  if (wksq < psq && file_of(wksq) == file_of(psq))

@ceebo
Copy link

ceebo commented Jun 10, 2018

@Rocky640 Sorry, you are right. I missed that wksq is also a relative square so everything does work out. I just saw a Bitboard being anded with a relative square and immediately assumed it as wrong.

@snicolet snicolet changed the title Simplify KRKP ending Optimize an expression in endgame.cpp Jun 11, 2018
@snicolet snicolet closed this in fc3af7c Jun 11, 2018
@snicolet
Copy link
Member

Merged via fc3af7c, thanks.

goodkov pushed a commit to goodkov/Stockfish that referenced this pull request Jul 21, 2018
I believe using foward_file_bb() here is fewer instructions.

a) Fewer instructions and probably more clear (debatable).
b) Possible that a lookup is slower than a few local operations, but the
   forward_file_bb table is probably used often enough that it is always
   cached.

Passed
LLR: 2.96 (-2.94,2.94) [-3.00,1.00]
Total: 21004 W: 4263 L: 4141 D: 12600
http://tests.stockfishchess.org/tests/view/5b1cad830ebc5902ab9c6239

Closes official-stockfish#1644

No functional change.
CounterPly pushed a commit to CounterPly/Stockfish that referenced this pull request Oct 16, 2018
I believe using foward_file_bb() here is fewer instructions.

a) Fewer instructions and probably more clear (debatable).
b) Possible that a lookup is slower than a few local operations, but the
   forward_file_bb table is probably used often enough that it is always
   cached.

Passed
LLR: 2.96 (-2.94,2.94) [-3.00,1.00]
Total: 21004 W: 4263 L: 4141 D: 12600
http://tests.stockfishchess.org/tests/view/5b1cad830ebc5902ab9c6239

Closes official-stockfish#1644

No functional change.
CounterPly pushed a commit to CounterPly/Stockfish that referenced this pull request Oct 16, 2018
I believe using foward_file_bb() here is fewer instructions.

a) Fewer instructions and probably more clear (debatable).
b) Possible that a lookup is slower than a few local operations, but the
   forward_file_bb table is probably used often enough that it is always
   cached.

Passed
LLR: 2.96 (-2.94,2.94) [-3.00,1.00]
Total: 21004 W: 4263 L: 4141 D: 12600
http://tests.stockfishchess.org/tests/view/5b1cad830ebc5902ab9c6239

Closes official-stockfish#1644

No functional change.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants