No, the MUL checks are in place explicitly to try to avoid every time into the DIV. I feel my hair growing back, thinking at the times that code made sense ☺
On Mon, Nov 30, 2015 at 7:46 PM, Dan Cross <[email protected]> wrote: > I don't think that's an optimization; it's an attempt to avoid UB. In that > sense, it's explicitly an anti-optimization.... > > On Mon, Nov 30, 2015 at 10:43 PM, 'Davide Libenzi' via Akaros < > [email protected]> wrote: > >> Oh, I would not have made any comment, had I not seen the silly >> optimization in place. Likely 1993 era 😀 >> No optimization in place, meant "we do not care at this point", which I >> agree in this case. >> But the code was suggesting "we do care", hence a much better way to do >> it. >> >> >> On Mon, Nov 30, 2015 at 7:41 PM, Dan Cross <[email protected]> wrote: >> >>> Also, bear in mind that this is right before we run the memory >>> allocator. Shaving a few cycles off here is kind of like chipping a pebble >>> off of an asteroid. >>> >>> On Mon, Nov 30, 2015 at 10:37 PM, Dan Cross <[email protected]> wrote: >>> >>>> Numbers of it didn't happen. >>>> >>>> On Mon, Nov 30, 2015 at 9:49 PM, 'Davide Libenzi' via Akaros < >>>> [email protected]> wrote: >>>> >>>>> The MUL_NO_OVERFLOW thing makes little sense in the CPUs we are >>>>> targeting. >>>>> A couple of mis-predicted branches are more expensive than a DIV (~20 >>>>> cycles in today Intel). >>>>> Much faster: >>>>> >>>>> typedef unsigned __int128 uint128_t; >>>>> >>>>> { >>>>> uint128_t n = (uint128_t) nmemb * (uint128_t) size; >>>>> >>>>> if (n > SIZE_MAX) foo(); >>>>> >>>>> } >>>>> >>>>> >>>>> -------------------- >>>>> typedef unsigned __int128 u128; >>>>> >>>>> static const unsigned long ulmax = ~0UL; >>>>> >>>>> unsigned long oc(unsigned long a, unsigned long b) >>>>> { >>>>> u128 m = (u128) a * (u128) b; >>>>> >>>>> if (m > ulmax) >>>>> m = 0; >>>>> >>>>> return (unsigned long) m; >>>>> } >>>>> >>>>> ---- >>>>> oc: >>>>> movq %rdi, %rax >>>>> mulq %rsi >>>>> movq %rax, %r9 >>>>> xorl %eax, %eax >>>>> testq %rdx, %rdx >>>>> cmove %r9, %rax >>>>> ret >>>>> >>>>> >>>>> >>>>> >>>>> >>>>> >>>>> >>>>> On Mon, Nov 30, 2015 at 1:18 PM, Barret Rhoden <[email protected]> >>>>> wrote: >>>>> >>>>>> One thing to be careful of is that these functions can fail and >>>>>> return 0, >>>>>> regardless of your kmalloc flags. >>>>>> >>>>>> Signed-off-by: Barret Rhoden <[email protected]> >>>>>> --- >>>>>> kern/include/kmalloc.h | 5 +++-- >>>>>> kern/src/kreallocarray.c | 9 +++++++++ >>>>>> 2 files changed, 12 insertions(+), 2 deletions(-) >>>>>> >>>>>> diff --git a/kern/include/kmalloc.h b/kern/include/kmalloc.h >>>>>> index 268cf2985ddb..26f23c1754c0 100644 >>>>>> --- a/kern/include/kmalloc.h >>>>>> +++ b/kern/include/kmalloc.h >>>>>> @@ -16,8 +16,9 @@ >>>>>> #define KMALLOC_LARGEST KMALLOC_SMALLEST << NUM_KMALLOC_CACHES >>>>>> >>>>>> void kmalloc_init(void); >>>>>> -void* kmalloc(size_t size, int flags); >>>>>> -void* kzmalloc(size_t size, int flags); >>>>>> +void *kmalloc(size_t size, int flags); >>>>>> +void *kmalloc_array(size_t nmemb, size_t size, int flags); >>>>>> +void *kzmalloc(size_t size, int flags); >>>>>> void *kmalloc_align(size_t size, int flags, size_t align); >>>>>> void *kzmalloc_align(size_t size, int flags, size_t align); >>>>>> void *krealloc(void *buf, size_t size, int flags); >>>>>> diff --git a/kern/src/kreallocarray.c b/kern/src/kreallocarray.c >>>>>> index d099721e0406..505159917317 100644 >>>>>> --- a/kern/src/kreallocarray.c >>>>>> +++ b/kern/src/kreallocarray.c >>>>>> @@ -34,3 +34,12 @@ void *kreallocarray(void *optr, size_t nmemb, >>>>>> size_t size, int flags) >>>>>> } >>>>>> return krealloc(optr, size * nmemb, flags); >>>>>> } >>>>>> + >>>>>> +void *kmalloc_array(size_t nmemb, size_t size, int flags) >>>>>> +{ >>>>>> + if (((nmemb >= MUL_NO_OVERFLOW) || (size >= MUL_NO_OVERFLOW)) >>>>>> && >>>>>> + (nmemb > 0) && ((SIZE_MAX / nmemb) < size)) { >>>>>> + return NULL; >>>>>> + } >>>>>> + return kmalloc(size * nmemb, flags); >>>>>> +} >>>>>> -- >>>>>> 2.6.0.rc2.230.g3dd15c0 >>>>>> >>>>>> -- >>>>>> You received this message because you are subscribed to the Google >>>>>> Groups "Akaros" group. >>>>>> To unsubscribe from this group and stop receiving emails from it, >>>>>> send an email to [email protected]. >>>>>> To post to this group, send email to [email protected]. >>>>>> For more options, visit https://groups.google.com/d/optout. >>>>>> >>>>> >>>>> -- >>>>> You received this message because you are subscribed to the Google >>>>> Groups "Akaros" group. >>>>> To unsubscribe from this group and stop receiving emails from it, send >>>>> an email to [email protected]. >>>>> To post to this group, send email to [email protected]. >>>>> For more options, visit https://groups.google.com/d/optout. >>>>> >>>> >>>> >>> -- >>> You received this message because you are subscribed to the Google >>> Groups "Akaros" group. >>> To unsubscribe from this group and stop receiving emails from it, send >>> an email to [email protected]. >>> To post to this group, send email to [email protected]. >>> For more options, visit https://groups.google.com/d/optout. >>> >> >> -- >> You received this message because you are subscribed to the Google Groups >> "Akaros" group. >> To unsubscribe from this group and stop receiving emails from it, send an >> email to [email protected]. >> To post to this group, send email to [email protected]. >> For more options, visit https://groups.google.com/d/optout. >> > > -- > You received this message because you are subscribed to the Google Groups > "Akaros" group. > To unsubscribe from this group and stop receiving emails from it, send an > email to [email protected]. > To post to this group, send email to [email protected]. > For more options, visit https://groups.google.com/d/optout. > -- You received this message because you are subscribed to the Google Groups "Akaros" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. To post to this group, send email to [email protected]. For more options, visit https://groups.google.com/d/optout.
