-
Notifications
You must be signed in to change notification settings - Fork 2.2k
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鈥檒l occasionally send you account related emails.
Already on GitHub? Sign in to your account
Packed like sardines TT #3
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -32,16 +32,15 @@ TranspositionTable TT; // Our global transposition table | |
|
||
void TranspositionTable::resize(uint64_t mbSize) { | ||
|
||
assert(msb((mbSize << 20) / sizeof(TTEntry)) < 32); | ||
uint32_t newClusters = 1 << msb((mbSize << 20) / sizeof(Cluster)); | ||
|
||
uint32_t size = ClusterSize << msb((mbSize << 20) / sizeof(TTEntry[ClusterSize])); | ||
|
||
if (hashMask == size - ClusterSize) | ||
if (newClusters == clusters) | ||
return; | ||
clusters = newClusters; | ||
|
||
hashMask = size - ClusterSize; | ||
hashMask = (clusters - 1) * sizeof(Cluster); | ||
free(mem); | ||
mem = calloc(size * sizeof(TTEntry) + CACHE_LINE_SIZE - 1, 1); | ||
mem = calloc(clusters * sizeof(Cluster) + CACHE_LINE_SIZE - 1, 1); | ||
|
||
if (!mem) | ||
{ | ||
|
@@ -60,7 +59,7 @@ void TranspositionTable::resize(uint64_t mbSize) { | |
|
||
void TranspositionTable::clear() { | ||
|
||
std::memset(table, 0, (hashMask + ClusterSize) * sizeof(TTEntry)); | ||
std::memset(table, 0, clusters * sizeof(Cluster)); | ||
} | ||
|
||
|
||
|
@@ -71,16 +70,12 @@ void TranspositionTable::clear() { | |
const TTEntry* TranspositionTable::probe(const Key key) const { | ||
|
||
TTEntry* tte = first_entry(key); | ||
uint32_t key32 = key >> 32; | ||
|
||
for (unsigned i = 0; i < ClusterSize; ++i, ++tte) | ||
if (tte->key32 == key32) | ||
{ | ||
tte->generation8 = generation; // Refresh | ||
return tte; | ||
} | ||
const uint16_t key16 = key >> 48; | ||
|
||
return NULL; | ||
if (tte->key != key16 && (++tte)->key != key16 && (++tte)->key != key16) | ||
return NULL; | ||
tte->genBound = generation | tte->genBound & 0x3; // Refresh | ||
return tte; | ||
} | ||
|
||
|
||
|
@@ -94,28 +89,49 @@ const TTEntry* TranspositionTable::probe(const Key key) const { | |
|
||
void TranspositionTable::store(const Key key, Value v, Bound b, Depth d, Move m, Value statV) { | ||
|
||
TTEntry *tte, *replace; | ||
uint32_t key32 = key >> 32; // Use the high 32 bits as key inside the cluster | ||
|
||
tte = replace = first_entry(key); | ||
TTEntry* tte = first_entry(key); | ||
const TTEntry* last = tte + ClusterSize - 1; | ||
TTEntry* replace = tte; | ||
uint16_t key16 = key >> 48; // Use the high 16 bits as key inside the cluster | ||
|
||
for (unsigned i = 0; i < ClusterSize; ++i, ++tte) | ||
for (;;) | ||
{ | ||
if (!tte->key32 || tte->key32 == key32) // Empty or overwrite old | ||
|
||
// Empty entry? | ||
if (!tte->key) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we should remove this if block. tte->key will be 0 a lot of the time for valid keys as well, so we could replace an entry with high depth here. But maybe this is worth a test against itself, as it will be difficult to measure the effect against normal SF. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, I agree. Good point. You are correct, |
||
{ | ||
if (!m) | ||
m = tte->move(); // Preserve any existing ttMove | ||
tte->key = key16; | ||
tte->move16 = (uint16_t)m; | ||
break; | ||
} | ||
|
||
replace = tte; | ||
// Overwrite old? | ||
if (tte->key == key16) | ||
{ | ||
if (m) | ||
// Only store move if there is one | ||
tte->move16 = (uint16_t)m; | ||
break; | ||
} | ||
|
||
if (tte == last) | ||
{ | ||
tte = replace; | ||
tte->key = key16; | ||
tte->move16 = (uint16_t)m; | ||
break; | ||
} | ||
|
||
// Implement replace strategy | ||
if ( ( tte->generation8 == generation || tte->bound() == BOUND_EXACT) | ||
- (replace->generation8 == generation) | ||
- (tte->depth16 < replace->depth16) < 0) | ||
++tte; | ||
// Is the next entry a better candidate for replacement? | ||
if ((tte->gen() == generation || tte->bound() == BOUND_EXACT) | ||
- (replace->gen() == generation) | ||
- (tte->depth8 < replace->depth8) < 0) | ||
replace = tte; | ||
} | ||
|
||
replace->save(key32, v, b, d, m, generation, statV); | ||
tte->value16 = (int16_t)v; | ||
tte->evalValue = (int16_t)statV; | ||
tte->depth8 = (uint8_t)(d - DEPTH_NONE); | ||
tte->genBound = generation | (uint8_t)b; | ||
} |
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 code leads to undefined behaviour. Never use several ++ operators on the same variable within an expression. Simply use tte[1] and tte[2] instead.
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.
@lucasart, that is incorrect. It doesn't lead to "undefined behavior". According to ISO/IEC C++ Standard 14882 搂5.14 Logical AND operator:
Also, I want
tte
pointing to the matching entry when/if the statement gets short-circuited. Usingtte[1]
andtte[2]
doesn't accomplish that.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.
Ah yes, you're right. The lazy boolean evaluation is guaranteed by the standard, so no compiler can screw up that code.