Re: [eigen] portable reallocation...

[Hauke Heibel <hauke.heibel@xxxxxxxxxxxxxx>]

On Sun, Feb 28, 2010 at 1:13 AM, Benoit Jacob <jacob.benoit.1@xxxxxxxxx> wrote:
> the posix_memalign case was unimplemented, I overlooked that when
> reviewing your patch. I did it, closely following your mm-realloc
> code, and I made a few changes in your code, and I have a few
> questions (...)

ups, thank you. ;)

> OK so here you were testing for (ptr!=0 && size==0) and I replaced
> that with just size==0.
>
> Why were you checking ptr!=0 here?
>
>
>   {
>     _mm_free(ptr);
> -    return NULL;
> +    return 0;
>   }

Well, maybe that is not really required. Actually, I just did it
because all default implementations I tested do something like this

if (ptr==0) return malloc(size);

and interestingly this is not necessarily 0, when size==0. But maybe
it is a good idea to enforce that behaviour

> One more question:
>
> in the line
>
> #if defined(_MSC_VER) && defined(_mm_free)
>
> Why are you checking for _mm_free ? Is it even guaranteed to be a
> preprocessor symbol? Why don't you just check for EIGEN_HAS_MM_MALLOC?
>
> Last question: in this code:
>
> #if defined(_MSC_VER) && defined(_mm_free)
>  result = _aligned_realloc(ptr,new_size,16);
> #else
>  result = ei_mm_realloc(ptr,new_size,old_size);
> #endif
>
>
> You mean that MSVC's _aligned_ functions are cool because they have
> realloc, whereas the _mm functions don't. Then shouldn't we just move
> MSVC's functions up in the priority list of functions to use? Is there
> any disadvantage in using them instead of _mm functions, when they are
> available? This would allow to remove this #ifdef here.

Somehow these are related. In MSVC (at least the newer versions),
_mm_malloc and _mm_free are implemented in terms of
_aligned_malloc/free -- they are indeed typedefs. I am just double
checking that with "defined(_mm_free)" since it might be that some old
versions are implemented differently.

Thinking of it again, I think it should be possible to consolidate the
three hand-made implementations
(ei_handmake_aligned_realloc/ei_mm_realloc/ei_posix_memalign_realloc)
into a single function. I have to double check that.

- Hauke