-
Notifications
You must be signed in to change notification settings - Fork 22
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
Use None=0 #144
base: master
Are you sure you want to change the base?
Use None=0 #144
Conversation
Allows some code simplifications at the expense of added complexity elsewhere. Overall it seems to be a size reduction. -13 bytes
} | ||
return str; | ||
} | ||
|
||
[[nodiscard]] int piece_on(const Position &pos, const int sq) { | ||
const u64 bb = 1ULL << sq; | ||
for (int i = 0; i < 6; ++i) { | ||
for (int i = 1; i < 7; ++i) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can't this run from 0 to 7? (maybe with a compiled out assert for the fallthrough)
@@ -391,7 +392,7 @@ const int pawn_attacked[] = {S(-64, -14), S(-155, -142)}; | |||
|
|||
// For each piece type | |||
for (int p = 0; p < 6; ++p) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why isn't this loop simply going from 1 to 7, avoiding all the p + 1
? I'd compute that once into a var for the eval array indexing, if only the avoid the eyebleed this is now.
@@ -487,18 +488,18 @@ const int pawn_attacked[] = {S(-64, -14), S(-155, -142)}; | |||
u64 hash = pos.flipped; | |||
|
|||
// Pieces | |||
for (int p = Pawn; p < None; p++) { | |||
for (int p = 1; p < 7; p++) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This could still use symbols.
u64 copy = pos.pieces[p] & pos.colour[0]; | ||
while (copy) { | ||
const int sq = lsb(copy); | ||
copy &= copy - 1; | ||
hash ^= keys[p * 64 + sq]; | ||
hash ^= keys[(p - 1) * 64 + sq]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can cheat and make keys bigger to avoid the -1.
@@ -424,7 +425,7 @@ const int pawn_attacked[] = {S(-64, -14), S(-155, -142)}; | |||
score += pawn_attacked[c]; | |||
} | |||
|
|||
if (p == Pawn) { | |||
if (p + 1 == Pawn) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeeaargh.
Code complexity for this is a bit mixed. Indexing based on piece type is now both awkward and inconsistent, but gains are made elsewhere. Would recommend we discuss if this is worthwhile and verify that it's correct. Happy to leave it until size reductions get desperate.