Open
Conversation
Owner
Malkovsky
commented
Apr 20, 2026
Malkovsky
commented
- Implemented 512-bit target excess positions based on 16-bit window expansion
- Implemented 512-bit target excess positions based on two-stage 4-bit lookups
3.6-5.5x faster than the 16-bit expansion approach (16-24 ns vs 86 ns per 512-bit block). Uses vpshufb for 4-bit LUT lookups, byte-level exclusive prefix sum on __m128i, and pdep for result interleaving.
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
| const size_t num_blocks = 4096; | ||
|
|
||
| std::mt19937_64 rng(42); | ||
| std::vector<std::array<uint64_t, 8>> blocks(num_blocks); |
Contributor
There was a problem hiding this comment.
SUGGESTION: Benchmark setup can be reused to reduce noise
The RNG fill and vector allocation happen for each benchmark invocation and each Arg value. Consider moving blocks (and the RNG seeding) to a static/fixture to reduce setup overhead and improve signal-to-noise in the measurements.
Contributor
Code Review SummaryStatus: No Issues Found | Recommendation: Merge Files Reviewed (2 files)
Reviewed by gpt-5.2-codex-20260114 · 294,719 tokens |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #39 +/- ##
==========================================
+ Coverage 86.35% 87.34% +0.98%
==========================================
Files 11 12 +1
Lines 2850 3136 +286
Branches 562 606 +44
==========================================
+ Hits 2461 2739 +278
- Misses 254 262 +8
Partials 135 135
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
…rks, add overflow boundary test - Add detailed comment explaining why int8 arithmetic in base + pos_j computation is safe despite the boundary value 128 wrapping to -128 (neither -128 nor 128 equals 0, so cmpeq_epi8 produces no false positive) - Refactor benchmarks to share block generation via make_blocks() helper - Add OverflowBoundary test covering all x in [-64,64] with varying prefix fills to exercise the int8 arithmetic boundary cases
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
1 participant
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.