feat(kernels): fold the out-of-range mask into SubtractWiden too - #25
Merged
Conversation
SubtractWiden arrived in #20 while the folded-mask overloads were in flight in #21, so the two crossed: every other add and subtract entry point can hand back a per-row out-of-range mask and the widening subtract could not. Nothing about it is special — it was simply not there to extend at the time. Adds the three overloads, one per widening step, so the mask surface is symmetric across add and subtract again. The 32-to-64 one gets a vectorised masked helper matching its bool-returning sibling; the other two are scalar loops, as the plain overloads are. The widening SIMD path is the fiddly one to get right, because each 32-bit chunk widens into two 64-bit vectors and so contributes mask bits for two half-chunks, at b0 and b0 + halfLanes. Swapping those two offsets still produces plausible-looking output, so the tests were checked against exactly that mutation and five of the eleven lengths fail under it — the ones long enough to reach the vector path. Benchmarks gain the widening pair, whose per-step lane bookkeeping is heavier than the same-width kernels already covered. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Extends the columnar arithmetic API so SpanAddKernel.SubtractWiden can return a per-row out-of-range bitmap (folded into the arithmetic pass), bringing widening subtraction to parity with the existing folded-mask add/subtract entry points.
Changes:
- Add three
SpanAddKernel.SubtractWidenoverloads that accept anoutOfRangeMaskspan (32→64, 64→128, 128→256). - Add a SIMD-backed
SubtractWidenSameScale32To64Maskedhelper mirroring the existingAddWidenSameScale32To64Maskedlane/bit layout. - Add equivalence tests (folded mask vs. kernel +
WriteOutOfRangeMask) and widen-pair benchmark coverage.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| tests/Clast.DatabaseDecimal.Tests/NullableColumnarTests.cs | Adds folded-mask equivalence tests for widening subtract across tier steps and lengths. |
| src/Clast.DatabaseDecimal/Arithmetic/SpanAddKernel.cs | Introduces folded-mask SubtractWiden overloads and a vectorized same-scale 32→64 masked helper. |
| benchmarks/Clast.DatabaseDecimal.Benchmarks/NullableColumnarBenchmarks.cs | Adds benchmark cases for the widening folded-mask add/subtract pair. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
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.
Closes a gap left by two PRs crossing in flight:
SubtractWidenlanded in #20 while the folded-mask overloads were in review in #21, so every add and subtract entry point can hand back a per-row out-of-range mask except the widening subtract. Nothing about it is special — it just was not there to extend at the time.What is in here
Three
SubtractWidenoverloads taking a mask span, one per widening step (32→64, 64→128, 128→256), mirroring theAddWidenset exactly. The 32→64 one gets a vectorised masked helper alongside its bool-returning sibling; the other two are scalar loops, as their plain counterparts are.The mask surface is symmetric across add and subtract again: 7 overloads each side.
On the SIMD helper
The widening vector path is the one worth looking at. Each 32-bit chunk widens into two 64-bit vectors, so a single loop step contributes mask bits for two half-chunks, at
b0andb0 + halfLanes. Swapping those two offsets produces output that still looks plausible — right count, right values, bits merely on the wrong rows.So rather than assume the tests covered it, I mutated the helper to swap them: 5 of the 11 lengths fail, which are exactly the ones long enough to reach the vector path (the shorter ones run the scalar tail). Restored, all 33 pass.
Verification
999 tests pass on net8.0 and net10.0; net472 builds clean. Each new overload is held against the plain kernel followed by
WriteOutOfRangeMask— same results, same mask, same count — across lengths 1, 3, 7, 63, 64, 65 and 200, so the vector tail and the mask's final partial word are both covered. Fixtures force a guaranteed out-of-range row so the mask comparison is never vacuous at the short lengths.Benchmarks gain the widening pair, since their per-step lane bookkeeping is heavier than the same-width kernels already covered.
🤖 Generated with Claude Code