Conversation
…load and remove memmove from hot patch.
|
|
||
| ZMIJ_INLINE void write_digits(char* buffer, uint64_t digits, | ||
| bool drop_leading_zero, const data&) noexcept { | ||
| digits = digits >> (drop_leading_zero * sizeof(digits)); |
There was a problem hiding this comment.
actually, you want to shift by 8 bits.
digits >>= drop_leading_zero * 8;
| unsigned lo, int num_digits, | ||
| int dec_exp) noexcept -> char* { | ||
| #if ZMIJ_USE_SSE || ZMIJ_USE_SSE4_1 | ||
| _mm_storeu_si128(reinterpret_cast<__m128i*>(buffer + 1), digits); |
There was a problem hiding this comment.
The 16 byte memcpy call is replaced by the appropriate sse-move operation by the compiler.
| #elif ZMIJ_USE_SSE | ||
| _mm_storeu_si128(reinterpret_cast<__m128i*>(buffer + drop_leading_zero), | ||
| digits); |
There was a problem hiding this comment.
This branch is currently unreachable for SSE2 because the if (!ZMIJ_USE_NEON && !ZMIJ_USE_SSE4_1) above returns first. Also, if made reachable, storing at buffer + drop_leading_zero doesn't seem equivalent to memmove(buffer, buffer + drop_leading_zero, ...): it shifts the output in the opposite direction.
| unsigned lo, int num_digits, | ||
| int dec_exp) noexcept -> char* { | ||
| #if ZMIJ_USE_SSE || ZMIJ_USE_SSE4_1 | ||
| _mm_storeu_si128(reinterpret_cast<__m128i*>(buffer + 1), digits); |
| ZMIJ_INLINE void write_digits(char* buffer, uint64_t digits, | ||
| bool drop_leading_zero, const data&) noexcept { | ||
| digits = digits >> (drop_leading_zero * sizeof(digits)); | ||
| memcpy(buffer, &digits, sizeof(digits)); | ||
| memmove(buffer, buffer + drop_leading_zero, sizeof(digits)); | ||
| } |
There was a problem hiding this comment.
This change is reasonable but it should be 8, not sizeof(digits) and you also need to handle big endian, something like
unsigned shift = unsigned(drop_leading_zero) * 8;
digits = is_big_endian ? digits << shift : digits >> shift;
memcpy(buffer, &digits, sizeof(digits));
There was a problem hiding this comment.
MSVC only optimizes memcpu in O2, not in O1 https://godbolt.org/z/Mbnr33K8M
There was a problem hiding this comment.
Then whoever cares about perf should compile with O2. I don't think we need to do any heroics for specific MSVC configurations.
There was a problem hiding this comment.
MSVC only optimizes memcpu in O2, not in O1 https://godbolt.org/z/Mbnr33K8M
To be more precise it is /Oi that is implied with /O2, and it can be also be overridden with #pragma intrinsic(memcpy) to optimize in any mode.
Still you should just compile with /O2 if you care about perf.
There was a problem hiding this comment.
just compile with
/O2if you care about perf.
I always compile code with maximum optimizations, but it's not the case here.
|
It would be nice to see benchmark results for claimed performance improvements. |
Dropping the leading '0' on the float fixed path stored eight digit bytes and then memmoved them back over themselves; shifting before the store does it in the register. ftoa-benchmark, Apple M5 Max, fixed-notation floats: 4.25ns -> 4.20ns (-1.3%, p = 0.0003). The mixed benchmark is unchanged, as only ~15% of its values format as fixed. Based on #151.
|
Merged a fixed version of the write_digits optimization in a24bf7f, thanks. |
5 participants
…load and remove memmove from hot patch.