Impossible to compile on macOS because of c++ standards

[nurv]

Currently on file device.cpp in lines 21 and 26 there is a reference to function aligned_alloc. Although the C++ defined in CMake is C++11, according with CLang website, that function is only available at C++17.

The problem is that increasing the C++ causes references to random_shuffle to break:

In file included from /Users/nurv/git/unbabel/marian-dev/src/data/corpus_base.h:16:
/Users/nurv/git/unbabel/marian-dev/src/data/dataset.h:113:25: error: no member named 'random_shuffle' in namespace 'std'
  void shuffle() { std::random_shuffle(examples_.begin(), examples_.end()); }
                   ~~~~~^
In file included from /Users/nurv/git/unbabel/marian-dev/src/data/corpus_sqlite.cpp:3:
In file included from /Users/nurv/git/unbabel/marian-dev/src/data/corpus_sqlite.h:15:
In file included from /Users/nurv/git/unbabel/marian-dev/src/data/corpus_base.h:16:
/Users/nurv/git/unbabel/marian-dev/src/data/dataset.h:113:25: error: no member named 'random_shuffle' in namespace 'std'
  void shuffle() { std::random_shuffle(examples_.begin(), examples_.end()); }
                   ~~~~~^
In file included from /Users/nurv/git/unbabel/marian-dev/src/data/corpus_base.cpp:3:
In file included from /Users/nurv/git/unbabel/marian-dev/src/data/corpus.h:15:
In file included from /Users/nurv/git/unbabel/marian-dev/src/data/corpus_base.h:16:
/Users/nurv/git/unbabel/marian-dev/src/data/dataset.h:113:25: error: no member named 'random_shuffle' in namespace 'std'
  void shuffle() { std::random_shuffle(examples_.begin(), examples_.end()); }
                   ~~~~~^

This is caused because random_shuffle was deprecated on C++14 and removed on C++17. This makes impossible to compile the code in macOS without changing the source code.

[emjotde]

@nurv Hi, can you specify your comment on aligned_alloc a bit? The links seem to point somewhere else.

@snukky Can we get rid of random_shuffle here? It really should not be used anymore.

[nurv]

@emjotde its proposal P0035R4 for C++17, that you can read in more detail here.

[emjotde]

Weird. I am seeing conflicting information.

http://en.cppreference.com/w/c/memory/aligned_alloc

http://en.cppreference.com/w/cpp/memory/c/aligned_alloc

Trying to make sense of it. The thing is, it compiles fine on GNU g++ with only C+11 enabled.

[emjotde]

If I understand correctly, std::aligned_alloc from <cstdlib> is C++17, while the C-version aligned_alloc from <stdlib.h> is C++11. Now that's messy.

We use the version from <stdlib.h> which would make it conform to the standard. Is Clang behind here?

[nurv]

I've been searching for a while, and I'm starting to suspect that this function is not implemented in macOS. I've open "stdlib.h" and the function is basically not defined there. I've also found this url where they mention that the libc doesn't not implement it.

[snukky]

@emjotde I removed last std::random_shuffle from Iris/MNIST examples.

[emjotde]

@nurv So does this work now for you with C++17 support enabled?

[nurv]

@emjotde Sorry, I've been on vacation. No, it doesn't. The problem here is that macos has a bad implementation of C11, and doesn't implement aligned_alloc. I've confirm this with LLVM developers.

Does the memory need to be allocated in a alignment fashion here? If it does, we can try c++17 std::aligned_alloc. If not, is it possible to replace it with malloc, or similar?

[kpu]

Do you need to free the memory allocated by aligned_alloc? Cause aligned_alloc is literally implemented by allocating size + alignment + metadata, rounding up the resulting pointer to alignment, and then adding some metadata before the pointer so free knows where the actual allocation started.

It not, it's easy to roll your own, as I have done with HugeMalloc to try to get huge-page aligned memory.

[emjotde]

Oh, I meant, does it work with C++17 now that we removed std::random_shuffle?

[emjotde]

I can add a compile time check later to for MacOS/C++17 that will switch to std::aligned_alloc.

[emjotde]

Where are we with this one right now? Are you still having problems?

[nurv]

I've built a container with docker, and run it from there. I gave up on running it on macOS.

[emjotde]

OK. So we will keep this open. I believe we do not need the aligned alloc for the CPU version and will try to get rid of that. We are also working on a Windows build were this is causing a bit of trouble.

[emjotde]

It seems replacing aligned_alloc(align, size) just with malloc(size) works perfectly fine. I will do this soon in master.

[kpu]

Hey, AVX2 and AVX512 need 32-byte and 64-byte respectively. Might I suggest posix_memalign? https://www.unix.com/man-page/osx/3/posix_memalign/