On Fri, Aug 1, 2014 at 3:49 PM, Jakub Jelinek <ja...@redhat.com> wrote: > On Thu, Jul 31, 2014 at 11:32:12PM +0300, Janne Blomqvist wrote: >> a while ago I committed a patch to trunk adding a function >> xmallocarray to libgfortran, which is a malloc wrapper like xmalloc >> but has two arguments and does an overflow check before multiplying >> them together. > > That seems to be unnecessarily expensive for the common case where both > nmemb and size are small. > > calloc in glibc uses something like following to avoid the division most of > the time, if both nmemb and size are small enough then nmemb * size can't > overflow. At least for 64-bit architectures small is smaller than 4GB > for both numbers. > > 2014-08-01 Jakub Jelinek <ja...@redhat.com> > > * runtime/memory.c (xmallocarray): Avoid division for the common case. > > --- libgfortran/runtime/memory.c.jj 2014-06-18 08:50:33.000000000 +0200 > +++ libgfortran/runtime/memory.c 2014-08-01 14:41:08.385856116 +0200 > @@ -56,7 +56,9 @@ xmallocarray (size_t nmemb, size_t size) > > if (!nmemb || !size) > size = nmemb = 1; > - else if (nmemb > SIZE_MAX / size) > +#define HALF_SIZE_T (((size_t) 1) << (__CHAR_BIT__ * sizeof (size_t) / 2)) > + else if (__builtin_expect ((nmemb | size) >= HALF_SIZE_T, 0) > + && nmemb > SIZE_MAX / size) > { > errno = ENOMEM; > os_error ("Integer overflow in xmallocarray");
Nice, though as os_error() has the _Noreturn specifier the __builtin_expect() is not necessary, right? In libgfortran.h we have in the comment block above the likely/unlikely macros, which are wrappers around __builtin_expect: "...as __builtin_expect overrides the compiler heuristic, do not use in conditions where one of the branches ends with a call to a function with __attribute__((noreturn)): the compiler internal heuristic will mark this branch as much less likely as unlikely() would do." Ok for trunk/4.9/4.8. If you choose to leave the __builtin_expect there, please explain why? -- Janne Blomqvist