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