[libcxx-commits] [PATCH] D128215: [libc++] Reduces std::to_chars instantiations.

[Mark de Wever via Phabricator via libcxx-commits]

Mordante added a comment.

In D128215#3597096 <https://reviews.llvm.org/D128215#3597096>, @philnik wrote:

>> Yes we still support 32-bit platforms, using a 64-bit value there would give a performance penalty, especially on platforms with a small number or registers.
>
> Isn't `size_t` 32 bits wide on 32-bit platforms?
>
>> Another reason why I prefer 32-bits is that I'm quite sure that division of a 32-bit value by a constant is more efficient than the division of a 64-bit value. (The compiler will transform it in a multiplication and a shift which IIRC always fits in a 64-bit register, when using a 32-bit value.) This is what we're doing for base 10.
>
> Could you do a benchmark to check if using a 32-bit wide instantiation yields better performance even on 64-bit platforms?

Actually I was wrong, for base 10 64-bit it's about as efficient. I test whether a 64-bit value fits in a 32-bit value and directly dispatch to a 32-bit value. So I guess that's why there's no penalty. But for the other bases there's a penalty. Several bases that were 25 ns before now are 30 ns. I haven't measured on 32-bit systems, but there I expect the penalty to be worse. So I will keep the 32 and 64 bit paths separated.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D128215/new/

https://reviews.llvm.org/D128215