naibaf7 / caffe

Caffe: a fast open framework for deep learning. With OpenCL and CUDA support.
http://caffe.berkeleyvision.org/
Other
85 stars 20 forks source link

Fixes required for OSX + clang 3.6.0 #4

Closed aharrison24 closed 9 years ago

aharrison24 commented 9 years ago

These commits contain changes necessary to build on OSX. I won't claim that they are 'correct' solutions to the problems I faced, but they should at least give pointers to what the problem is, and you can then come up with better fixes where appropriate.

Notes:

naibaf7 commented 9 years ago

I am not quite sure why you (have to?) exclude certain explicit instantiations? They are supposed to work like they were...

aharrison24 commented 9 years ago

Here's a flavour of the linker errors I get with clang (both 3.6.0 and the 'default' XCode clang): https://gist.github.com/aharrison24/a38a5ff417b5c67f0247

I haven't been able to find much on this through googling, but this link gives a hint that maybe duplicate explicit instantiation is an 'extension' allowed by MSVC (and clang when in MSVC compatibility mode, as it normally is on Windows platforms). http://lists.cs.uiuc.edu/pipermail/cfe-commits/Week-of-Mon-20140505/105205.html

The C++ Standard appears to be quite clear on this (though I'm well out of my depth here): 14.7.5.1 For a given template and a given set of template-arguments, an explicit instantiation definition shall appear at most once in a program,

(You can find this at the bottom of page 379: http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2014/n4296.pdf )

On 7 July 2015 at 06:58, Fabian Tschopp notifications@github.com wrote:

I am not quite sure why you (have to?) exclude certain explicit instantiations? They are supposed to work like they were...

— Reply to this email directly or view it on GitHub https://github.com/naibaf7/caffe/pull/4#issuecomment-119073786.

aharrison24 commented 9 years ago

Just to be clear, I'm compiling without CUDA, so USE_CUDA is false and USE_GREENTEA is true. Which means the .cu files are being 'included' into the .cpp files. The result is that the invocations of 'INSTANTIATE_CLASS' and 'INSTANTIATE_LAYER_GPU_FUNCS' are happening in the same compilation unit. In the upstream repo, those macros don't get used together in a single compilation unit.

aharrison24 commented 9 years ago

Rather than having blocks like:

if defined(USE_GREENTEA) && !defined(USE_CUDA)

include "power_layer.cu"

endif

...perhaps the '.cu' files should just be compiled standalone and linked as usual, as if they were cpp files. That way you could let the linker remove the dupes and you wouldn't need the preprocessor trickery.

Important caveat if you're doing this in CMake: http://stackoverflow.com/questions/18784811/cmake-and-non-standard-extensions

naibaf7 commented 9 years ago

@aharrison24 I fixed .cu compilation now. Try if it works for you.

aharrison24 commented 9 years ago

Yes, all seems good now.

Still can't build the 'runtest' target though, because TEST_DEVICE isn't defined for the CMake build.

On 7 July 2015 at 19:23, Fabian Tschopp notifications@github.com wrote:

@aharrison24 https://github.com/aharrison24 I fixed .cu compilation now. Try if it works for you.

— Reply to this email directly or view it on GitHub https://github.com/naibaf7/caffe/pull/4#issuecomment-119274831.

naibaf7 commented 9 years ago

@aharrison24 OK, that seems to be an inherited bug from the original caffe, or should be easy enough to fix either way. I'll look into it when I find time. Or you can fix it if you want ;)