Closed abergeron closed 11 years ago
I've manually looked at these diffs, and they look good to me. I noticed a lot of the same changes that I have been making in my work to integrate #14, like removing the unnecessary
How did you get cmake to detect your installed OpenCL? I do not see any edits to the cmake logic.
I have made a request at work to acquire apple hardware, as the interest in getting clMath to function on MacOS appears to be high. Until that happens, I am going to ask the community to help me test Mac specific pull-requests like this. @pavanky, @fommil, if you have time, could you download this pull request and tell me your experiences compiling on your respective mac platforms?
Since pull-requests #18 and #14 will conflict on merge and have the same goal, i would like to proceed with this one going forward as the warning fixes provide additional value.
I did not have to do anything special to get it to recognize the system one. CMake knows to look for frameworks on OS X and usually does the Right Thing (tm).
I have not tried with a third party runtime since I never got one to compile on OS X.
However I have seen in the comments for the other PR that there may be trouble with headers on 10.8. Since my development machine is 10.7 I can't tell exactly what the problem is.
@abergeron @fommil I figured out last night why my wife's laptop does not have have the OpenCL header files in the /System/Library/Frameworks/OpenCL.framework. I installed Xcode to the laptop, and then began generating the xcode project with cmake. However, just installing Xcode does not appear to install system header files. Inside of the Xcode IDE, there is an option to download and install 'command line tools'. After doing so, the command line tools actually installed the OpenCL headers into /System/Library/Frameworks/OpenCL.framework.
With that, the cmake logic to detect OpenCL worked as is and I was able to compile the library on the MacOSX machine.
I have one request for @abergeron, could you squash the 6 commits above into 2 commits, one to fix the OSX builds and 1 to fix the warnings that you fixed. Some of the commits only changed 1 line, and I believe that d43574f, 0b3696a, 292c37d, d33af62, c720414 could logically be combined into 1 commit for a cleaner branch history.
If you have problems squashing the commits, just tell me and I will merge as is, I just want to attempt to keep the history shorter if possible.
Squashing is done. Sorry for the delay.
@abergeron @kknox thanks for this! Sorry but I won't be able to test this for a while as I'm going on holiday and this week is crazy-busy with deliverables before I shoot off. I'll get a chance to look at it in more detail in October on OS X Mountain Lion.
This allows the library to build on Mac OS X (10.7 tested). The tests may not build since I haven't tried those yet.
I also fixed a lot of build warnings, some of which could lead to serious problems on 64-bit archs.