PR #369 gated the digit-separator/prefix feature on a compile-time
has_separator flag and claimed the has_separator==false instantiation
"compiles to exactly the same code as if the feature did not exist".
Instruction-count measurement (benchmark i/f, deterministic +/- 0.0%)
disproved this: routing the default path through the shared, restructured
body cost GCC 14 ~1.5-2.3 i/f on short doubles, a reproducible regression
on mesh/double (+0.77 i/f ASCII, +1.35 UTF-16) even as canada improved.
Fix: the has_separator==false instantiation now delegates to
parse_number_string_nosep, a verbatim copy of the original separator-free
scanner, so the default path's codegen is byte-for-byte the pre-feature
parser. The shared parse_number_string body becomes separator-only (the
dead !has_separator branches are removed). The inaccurate doc comment is
corrected.
Measured vs pre-feature baseline (34164f5), instructions per float:
canada/f64 ASCII 242.32 -> 238.87 (PR: 240.87)
canada/f64 UTF16 251.05 -> 241.81 (PR: 245.79)
mesh/f64 ASCII 107.69 -> 107.57 (PR: 108.46, +0.77 regression)
mesh/f64 UTF16 110.10 -> 109.74 (PR: 111.45, +1.35 regression)
All eight fastfloat rows now match or beat both the baseline and the PR;
the mesh/double regression is eliminated. All 14 ctest suites pass.
PR #369's "performance-neutral" claim did not hold: the default
(no-separator) parse was ~70% larger in the hot frame (parse_number_string
inlined into from_chars: 833 -> 1414 x86/arm64 instructions), because
1. parse_number_string is force-inlined (always_inline), and the runtime
dispatch inlined BOTH the has_separator==true and ==false instantiations
into the default caller -- dragging the entire separator-aware scanner
into the hot frame; and
2. the runtime separator branch survived in from_chars() even though a
default-constructed parse_options provably has no separator, because the
thin from_chars_caller forwarder was not inlined, so the compile-time
'\0' was lost across the call boundary and the branch (plus two calls)
could not be folded away.
Fixes:
* FASTFLOAT_NOINLINE + a cold parse_number_string_with_separator trampoline
so the separator instantiation stays strictly out of line; the default
caller only ever materializes the has_separator==false code.
* Force-inline from_chars_caller::call (all three specializations) so the
separator branch constant-folds away on the common from_chars() path.
Net: the default hot frame returns to 844 instructions (vs 833 on main; the
+11 is the unrelated parse_number_string refactor and is unchanged by this
commit). The separator path is unaffected and all tests pass.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Re-implements the optional digit-separator and base-prefix parsing
(originally PR #369) on top of the current store_spans hot-path
architecture, with the goal of zero overhead when the features are not
used.
Key changes vs. the original PR:
- has_separator is now a *compile-time* template parameter on
parse_number_string, dispatched once (from_chars_advanced ->
parse_number_string_options) based on options.digit_separator. The
has_separator==false instantiation that every default caller uses is
byte-for-byte the separator-free parser: no separator comparison ever
enters a digit loop and the SIMD eight-digit fast path is preserved.
The separator-aware code lives only in the cold true instantiation.
- The store_spans no-span hot path (added to main after the original PR
was branched) is preserved.
- parse_options_t fields are ordered so the two new single-byte fields
(digit_separator, format_options) fall into the existing padding for
UC == char. sizeof(parse_options_t<char>) stays 16 bytes, so the
struct is still register-passed and the call boundary is unchanged.
Result: for the default (no separator, no prefix) path, the generated
assembly of from_chars<double> is identical to main, and benchmarks show
no measurable regression (the original PR was 5-10% slower).
Adds basictest coverage for digit separators (including the >19-digit
overflow re-scan paths) and prefix skipping.
The previous commit detects multi-wrap u64 overflow at the max_digits
boundary by re-parsing the digits through a checked multiply-add loop
(O(max_digits)). Replace that with the constant-time check used in
simdjson: the leading digit plus a single threshold comparison.
For a max_digits-length value, min_safe_u64(base) == base^(max_digits-1)
is the smallest such value and also the width of each leading-digit band
[d*ms, (d+1)*ms). Since that width is < 2^64, the only band that can
straddle 2^64 is d == dmax (the largest leading digit that still fits),
and there it straddles at most once, so a single threshold dmax*ms
separates wrapped from non-wrapped values. A leading digit above dmax
always overflows; below dmax always fits. dmax and the threshold derive
from the existing min_safe_u64 table, so no new tables are needed and
dmax*ms cannot itself overflow.
Add a programmatic, self-verifying test for parse_int_string overflow
detection covering bases 2..36, complementing the hand-picked strings
added earlier. Every generated input is cross-checked against an
independent trusted oracle (a plain 64-bit checked multiply-add); on
success the parsed value is also compared exactly and full consumption
of the input is asserted.
Per base it exercises:
- an exact-boundary sweep of the 64 values straddling 2^64
(UINT64_MAX-31 .. 2^64+31), built by walking the digit string;
- UINT64_MAX, 2^64 and the all-max-digit value, each also with
leading zeros;
- random max_digits-length values across every leading digit, with
the heaviest sampling on the lead == dmax band that straddles 2^64,
and full coverage of lead > dmax (the multi-wrap region the naive
min_safe check accepted by mistake);
- max_digits-1 (never overflows) and max_digits+1 (always overflows).
A small signed (int64_t) section checks the exact INT64_MIN/INT64_MAX
limits round-trip and that INT64_MAX+1 / INT64_MIN-1 are rejected in
every base.
This fixes a compile error in all clang versions lower than 10,
triggered by the use of the pragma ignore with what is an unknown
warning on those compiler versions:
```
/__w/ext/fast_float/include/fast_float/parse_number.h:361:34: error: unknown warning group '-Wc++20-extensions', ignored [-Werror,-Wunknown-pragmas]
```
The fix requires looking at __clang_major__, which is unfortunately
different in Apple, so a version dispatch is required.
parsed_number_string_t carries two span<UC const> members (integer, fraction)
that are only read on the rare slow paths (digit_comp, and the >19-significant-
digit truncation recompute). Materializing them on every parse forces the ~56/64-
byte struct to be written out and marshaled through the by-value return, which
shows up as backend/store pressure on the hot path.
This adds a runtime `store_spans` flag (default true, so all existing callers are
unchanged) to parse_number_string; from_chars_float_advanced parses with it false,
attempts the Clinger and Eisel-Lemire fast paths inline, and only re-parses with
spans on the two rare slow branches. The re-parse is pushed into a single
`fastfloat_noinline` (noinline+cold) helper so the force-inlined hot scanner is
emitted once rather than duplicated into the caller (without this the extra inline
copies regress some targets, e.g. ARM gcc, by bloating the hot frame and lengthening
the loop-carried dependency chain).
A runtime flag is used deliberately rather than a template parameter: a template
would create a second instantiation of the whole scanner whose icache cost wipes
out the gain.
Measured (per-parser microbench, median of 5, pinned core), fast_float from_chars
<double>/<float>, vs the current tip:
- Intel Ice Lake (Xeon 8360Y): +17-19% (gcc), Intel TMA shows backend-bound
26.0% -> 2.2% and retiring 60.3% -> 77.3% on short floats (the eliminated span
spill), with -36% pipeline slots.
- Intel Cascade Lake (Xeon 6248): +18-22% (gcc), +13-23% (clang).
- ARM Neoverse-V2 (Graviton4): +73-196% (gcc), +8-11% (clang) -- the struct spill
dominated the gcc hot loop there.
Correctness: the full float exhaustive suite (exhaustive32, exhaustive32_64,
exhaustive32_midpoint, random64) passes, and a 2^32 sweep is byte-identical to the
current tip. Public from_chars / from_chars_advanced / parsed_number_string_t are
unchanged.
Same std::thread split as exhaustive32_midpoint; preserves each test's existing
failure behavior (abort for exhaustive32, stop-flag for exhaustive32_64).
After the 8-digit SWAR block loop, consume a remaining 4-7 digit run in one
read4_to_u32 + parse_four_digits_unrolled step instead of byte-by-byte (reusing
the existing 4-digit helpers). The parsed result is identical; this is purely a
faster way to consume the same digits.
Gated to clang: on gcc the extra 4-digit check regresses inputs whose remainder
is < 4 digits (e.g. the 17-digit fraction of uniform [0,1] -> -3% on 'random'),
because the check becomes pure overhead there; clang does not show that.
m8g.metal-24xl (Graviton4), -O3 -march=native, simple_fastfloat_benchmark,
from_chars->double, clang 18, base vs patch back-to-back (2 samples):
canada.txt +11.7%, mesh.txt +7.4%, random ~flat. No regression.
parse_number_string scans the integer part one byte at a time in a while loop,
while the fraction already uses the 8-digit SWAR loop. Most integer parts are
1-5 digits, so the loop back-edge dominates. Peel the first five iterations into
nested ifs, falling through to the original while for longer runs. Semantics are
identical (i = 10*i + digit, advancing p); no behavior change.
AWS m8g.metal-24xl (Graviton4), -O3 -march=native, simple_fastfloat_benchmark,
from_chars->double. base vs patch measured back-to-back, mean of 2 runs:
canada: gcc +3.1%, clang +2.8%
mesh: gcc +5.4%, clang +5.1%
random: ~flat (1-digit integer part)
No regression; gcc and clang agree.
Alternatives benchmarked and rejected: reusing loop_parse_if_eight_digits for the
integer part regressed 5-8% (integer parts are too short for 8-digit SWAR setup);
a counted for(k<5) loop matched on gcc but clang optimized it worse (canada -0.9%).
The explicit peel is the only form solidly positive on both compilers.
Replaces uses of std::min with ternary operators in ascii_number.h, digit_comparison.h, and float_common.h to remove the dependency on the <algorithm> header in those files.
