On Tue, 23 Mar 2021 13:14:15 +0100
Andrew Jones <drjones@xxxxxxxxxx> wrote:
Hi,
> On Mon, Mar 22, 2021 at 09:35:23AM +0100, Andrew Jones wrote:
> > @@ -208,23 +209,46 @@ unsigned long int strtoul(const char *nptr, char **endptr, int base)
> > c = *s - 'A' + 10;
> > else
> > break;
> > - acc = acc * base + c;
> > +
> > + if (is_signed) {
> > + long __acc = (long)acc;
> > + overflow = __builtin_smull_overflow(__acc, base, &__acc);
> > + assert(!overflow);
> > + overflow = __builtin_saddl_overflow(__acc, c, &__acc);
> > + assert(!overflow);
> > + acc = (unsigned long)__acc;
> > + } else {
> > + overflow = __builtin_umull_overflow(acc, base, &acc);
> > + assert(!overflow);
> > + overflow = __builtin_uaddl_overflow(acc, c, &acc);
> > + assert(!overflow);
> > + }
> > +
>
> Unfortunately my use of these builtins isn't loved by older compilers,
> like the one used by the build-centos7 pipeline in our gitlab CI. I
> could wrap them in an #if GCC_VERSION >= 50100 and just have the old
> 'acc = acc * base + c' as the fallback, but that's not pretty and
> would also mean that clang would use the fallback too. Maybe we can
> try and make our compiler.h more fancy in order to provide a
> COMPILER_HAS_GENERIC_BUILTIN_OVERFLOW define like linux does for
> both gcc and clang. Or, we could just forgot the overflow checking.
In line with my email from yesterday:
Before we go down the path of all evil (premature optimisation!), can't
we just copy
https://git.kernel.org/pub/scm/libs/klibc/klibc.git/tree/usr/klibc/strntoumax.c
and have a tested version that works everywhere? This is BSD/GPL dual
licensed, IIUC.
I don't really see the reason to performance optimise strtol in the
context of kvm-unit-tests.
Cheers,
Andre
