Re: [PATCH, x86] Fix posix_memalign declaration in mm_malloc.h

[Marc Glisse <marc dot glisse at inria dot fr>]

On Fri, 13 Nov 2015, Bernd Schmidt wrote:

> On 11/13/2015 06:11 PM, Szabolcs Nagy wrote:
> > Followup to https://gcc.gnu.org/ml/gcc-patches/2015-05/msg01433.html
> >
> > The posix_memalign declaration is incompatible with musl libc in C++,
> > because of the exception specification (matters with -std=c++11
> > -pedantic-errors).  It also pollutes the namespace and lacks protection
> > against a potential macro definition that is allowed by POSIX.  The
> > fix avoids source level namespace pollution but retains the dependency
> > on the posix_memalign extern libc symbol.
>
> >   #ifndef __cplusplus
> > -extern int posix_memalign (void **, size_t, size_t);
> > +extern int _mm_posix_memalign (void **, size_t, size_t)
> >   #else
> > -extern "C" int posix_memalign (void **, size_t, size_t) throw ();
> > +extern "C" int _mm_posix_memalign (void **, size_t, size_t) throw ()
> >   #endif
> > +__asm__("posix_memalign");
>
> Can't say I like the __asm__ after the #if/#else/#endif block.

It might also cause trouble if some systems like to prepend an underscore, 
maybe?

> >   static __inline void *
> >   _mm_malloc (size_t size, size_t alignment)
> > @@ -42,7 +43,7 @@ _mm_malloc (size_t size, size_t alignment)
> >       return malloc (size);
> >     if (alignment == 2 || (sizeof (void *) == 8 && alignment == 4))
> >       alignment = sizeof (void *);
> > -  if (posix_memalign (&ptr, alignment, size) == 0)
> > +  if (_mm_posix_memalign (&ptr, alignment, size) == 0)
> >       return ptr;
> >     else
> >       return NULL;
>
> Random observation: this seems overly conservative as malloc is defined to 
> return something aligned to more than one byte.

What do we assume in the compiler? MALLOC_ABI_ALIGNMENT seems to be be the 
default BITS_PER_WORD on x86 (or I grepped badly), which also seems quite 
conservative but not as much.

> Couldn't this bug be fixed by either
> - just overallocating and aligning manually (eliminating the dependence
>   entirely)

gmm_malloc.h does that, the whole point of pmm_malloc.h is to avoid it.

> - or moving _mm_malloc into libgcc.a?

--
Marc Glisse