Repository navigation
Conversation
A magic probe is a mask, a multiply, a shift and a lookup, and the lookup carried a bounds check the rest of it did not need. `offsets[i] + index` is a number llvm cannot bound, since `index` is a hash of the occupancy and the shift it is taken at is a runtime load, so the compare against the table length and the jump past it were emitted at every probe. The mobility count takes a probe at every slider of both sides at every leaf, which is where the engine pays for that most often. The build has already done the check. `SliderTables::new` enumerates the subsets of each square's blocker mask, which is exactly the set of values `occupied & mask` can take at a probe, and writes a move set to the slot each one indexes. That write is an ordinary checked index in a const function, so const evaluation bounds checked all 107,648 of them while the table was being built. A probe can only land on a slot that write already reached, so `get_unchecked` reads what the build proved was there. The square is masked to the board the way `Board::get_piece_index` masks it, with a debug assert over the mask, so `i` names one of the sixty four blocks whatever byte arrives. Nothing reaches here with a square off the board today. The one that could is the king square, since `king_index` answers 64 on a board with no king, and every path that probes from it reads a bounds checked attack mask at the same square first. So the mask takes away a backstop nothing uses and the debug assert keeps the half of it that would catch a new caller. The engine crate carries unsafe already and the crate that speaks the protocol forbids it, which is the boundary this stays on the right side of. This is the eleventh unsafe block and the third whose failure would be undefined behaviour rather than a wrong answer, so the manifest's inventory and the roadmap's Miri bullet both say so now. Instructions on `bench 5` go 450,153,896 to 445,509,184, 1.03% fewer, with 2,895,293 fewer conditional branches, which is the check itself, and 181,030 fewer mispredicts. First level and last level misses are flat to three digits, which is what a change that moves no table should read. The node count does not move, on the short bench or the full one. Five interleaved rounds of a depth ten bench read -0.0% between the medians and -0.1% between the fastest rounds, against a spread of 5.9%. Two identical binaries over the same rounds read -0.0% and -0.4%. So the clock cannot separate a change this size from a null on this box and says nothing in either direction. It does separate this and the commit above it taken together, at +1.6% on both columns. The count is what this rests on. Speed: -0.0% (bench nps, 5 interleaved rounds vs 378c148, spread 5.9%) Bench: 3868987 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`mobility::fold_with` takes its weights as `&[i32; COUNTS]` so that the tests can hand it their own, and `fold` passes `&MOBILITY`. Under `#[inline]` llvm left it out of line, and then the four multiplies that finish the term read that array through a pointer at every leaf instead of being the four numbers it holds. `#[inline(always)]` puts it in the one caller that ships, where the constants propagate: the multiplies are `imul $0x30006` and the like now, and `MOBILITY` is never loaded. `counts_of` has carried the same attribute for a different reason, which its own comment gives. There llvm keeps the body out of line and the cost is the call. Instructions on `bench 5` go 445,509,184 to 442,032,186, 0.78% fewer. The node count does not move. This is the third thing tried on the mobility term and the only one that helped, so what the other two measured is worth writing down. Walking the two colours together, so that the occupancy and the piece boards are read once rather than twice, was 0.32% worse: llvm already shares those reads across the two calls, and interleaving the colours costs more in registers than the sharing saves. Accumulating each kind's weighted difference in place of the two count arrays was 0.04% worse. The loop the term already had is what the count says to keep, and the weights were the only thing in it being fetched that did not have to be. Five interleaved rounds of a depth ten bench read -0.2% between the medians and +3.0% between the fastest rounds, against a spread of 6.9%, which is a reading that says nothing. Taken with the commit below it the clock reads +1.6% on both columns, against a null of -0.0% and -0.4%. Speed: -0.2% (bench nps, 5 interleaved rounds vs 74a3e26, spread 6.9%) Bench: 3868987 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The budget bullet's 8.48% predates the refit that priced all four mobility kinds, and it reads low. Measured again at `378c148` the term is 60,880,488 instructions of 450,153,896 on `bench 5`, which is 13.52%, so it is further over the 5% than the bullet said. The two commits below charge it and leave it well over, so what the bullet asks is unchanged. The four arms this round measured and did not carry go in the rejected list, since the answer to that bullet is not going to be a faster loop and the next round should not spend itself finding that out again. Two were restructures of the mobility count. One was the masking that pays in the magic attack table, applied to three reads that had no bounds check to remove. One was the attribute the commit below carries, on the two terms that sit behind a cache. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Contributor
Speed against
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
A round of speed work aimed at the mobility term, which
docs/ROADMAP.mdrecords as over the 5% a leaf term is allowed. Two changes, 1.80% of the
bench between them, node count unmoved at 3,868,987.
perf(magic)drops the bounds check from the slider attack lookup. The indexis a hash of the occupancy so llvm cannot bound it, and the compare and the
jump past it were emitted at every probe. What makes the unchecked read sound
is that
SliderTables::newhas already written every slot a probe can reach,through a checked index, in const evaluation. 1.03% fewer instructions and
2.9 million fewer conditional branches, with cache misses flat.
perf(eval)forcesmobility::fold_withinline. Out of line it read thefour weights through a pointer at every leaf; inlined they propagate as
constants. 0.78% fewer instructions.
The term itself gave nothing else up. Four arms were measured and dropped:
walking the two colours together, accumulating weighted counts in place of
the count arrays, the same masking applied to three other square indexed
reads, and the same inline attribute on the two terms that sit behind a
cache. All four are in the roadmap's rejected list so the next round does not
spend itself on them again. The budget bullet's 8.48% is corrected there too.
The term is 13.52% of
bench 5at the base of this branch, and these twocommits leave it a long way over the 5%.
On the clock, neither perf commit can be separated from a null on this box.
Five interleaved rounds of a depth ten bench read -0.0% and -0.2%
individually, against a null of -0.0%. The pair reads +1.6% on both the
median and the fastest round. The instruction count is what these rest on,
the way the swap and the ordering commits were read.
The runner is quieter than this box, and its own nine rounds read the pair at
+1.0% between the medians and +1.5% between the fastest rounds, against a
spread of 2.8%, with 72 of the 81 base against candidate pairs favouring the
candidate. That is the comment below, and it is the better of the two clock
readings.
What is left for the budget is a cheaper formulation of the term, or not
paying for it at every leaf. #250
measured the cache under the position key and it cost time while saving
instructions. Two arms this round did not take: a lazy skip when the partial
score is far enough outside the window that mobility cannot move it across,
which changes the tree and wants an sprt, and computing each side's slider
attacks once per node for both the evaluation and capture generation, which
is a question about the board and eval seam rather than a tweak.
🤖 Generated with Claude Code