Re: [eigen] portable reallocation...

[Benoit Jacob <jacob.benoit.1@xxxxxxxxx>]

Hi,

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 (if only to check that I didn't introduce regressions in
your code!!). Here's part of the diff (changeset e6f4fb4a41aa), I put
some questions inline:

-  // 0. Check if size==0 and act according to the standard.
-  if (ptr!=0 && size==0)
+  // 0. Check if size==0 and act according to the standard, which says that
+  // for size==0, the object pointer (i.e. ptr) should be freed.
+  if (size==0)


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;
   }


As you can see I replaced NULL by 0 for homogeneity with the rest of Eigen code.


   // 1. Allocate new memory
   void* newptr = _mm_malloc(size,16);

   // 2. Verify the allocation success
-  // Testing for size!=0 is important since the standard says that
-  // for size==0, the object pointer (i.e. ptr) should be freed.


As you can see, I moved that comment above. It didn't seem to make sense here?

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.

Cheers,
Benoit