Identify AppleClang as a valid Clang compiler #3131

SergioRAgostinho · 2019-06-06T14:15:04Z

https://cmake.org/cmake/help/v3.5/variable/CMAKE_LANG_COMPILER_ID.html

I just printed some make verbose and noticed there were no SSE flags.

SunBlack · 2019-06-06T14:19:13Z

I don't know if PCL is supporting ARM, but CMake 3.15 will introduce ARMClang. Maybe we should add this, too.

//Edit: Your commit message contains a typo (just in case you want to fix it before merge)

SergioRAgostinho · 2019-06-06T14:24:06Z

No opinion against.

taketwo · 2019-06-06T14:27:01Z

I noticed the following piece of code in the main CMakeLists:

pcl/CMakeLists.txt

Lines 191 to 193 in 575a9b9

    
               if(APPLE AND WITH_CUDA AND CUDA_FOUND) 
        
                 set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -stdlib=libstdc++") 
        
               endif()

It is nested under an if that checks for Clang. So currently this is never executed. After merging your fix, -stdlib=libstdc++ will get appended on Mac. Do you know why? Do we need this given that this did not work as expected before?

SergioRAgostinho · 2019-06-06T14:31:13Z

Do you know why? Do we need this given that this did not work as expected before?

The only reason to do so is if you're depending on a 3rd party which was compiled with libstdc++. That's not the default case on OS X. I suspect this is a remnant from the macports age. These days there's no reason to unconditionally set the standard and if there is one, libc++ should be the default on OS X.

Edit: I just noticed the CUDA there... this is an NVCC issue. I don't have a way to test this at the moment. and I'm reading mixed reports from here. I suspect this should no longer be an issue in more recent nvcc versions.

taketwo · 2019-06-06T14:42:28Z

Do you have any experts in mind whom we could summon to get an opinion?

SergioRAgostinho · 2019-06-06T14:46:04Z

@claudiofantacci do you still have a working cuda toolkit on your mac? Do you mind testing if:

currently you can build pcl CUDA code (with the current master branch)
if -stdlib=libstdc++ is being added or not as a flag

claudiofantacci · 2019-06-06T19:57:56Z

@SergioRAgostinho yes I do 🚀

I'll try it out tomorrow and should you not hear anything from me anytime soon, just ping me 👍

SergioRAgostinho · 2019-06-07T08:35:32Z

Thank you.

It feels like we are dealing with same issue in both. The std=c++14 flag is likely not being passed to nvcc. Something that needs to be fixed first in order to test the original case further.

Edit: There's also the issue of has having to propagate the same user selected standard from the host compiler (clang/gcc) to nvcc, i.e., a user can select c++17 and that should probably be consistent between both compilers.

claudiofantacci · 2019-06-07T08:53:35Z

It feels like we are dealing with same issue in both. The std=c++14 flag is likely not being passed to nvcc. Something that needs to be fixed first in order to test the original case further.

By my experience, that kind of flag has always been a pain on UNIX systems.
I usually add something like this in my projects

if (UNIX)
  list(APPEND CUDA_NVCC_FLAGS "-std=c++11")
endif()

We could, of course, do something smarter by, e.g., get the flag from some CMake variable.

SunBlack · 2019-06-07T11:03:02Z

We could, of course, do something smarter by, e.g., get the flag from some CMake variable.

There exists a smarter way... starting from CMake 3.8 (we are limited to 3.5 because of Ubuntu 16.04) ;)

claudiofantacci · 2019-06-07T11:15:15Z

There exists a smarter way... starting from CMake 3.8 (we are limited to 3.5 because of Ubuntu 16.04) ;)

Still, the variable seems to be used only if you add CUDA as a language of the project. The variable may not work properly if you do find_package(CUDA) (which unfortunately is still necessary if you want CMake to create IDE projects).

claudiofantacci · 2019-06-07T11:15:42Z

But anyway, good to know! Was unaware of the CUDA_STANDARD variable 👍

SunBlack · 2019-06-07T11:26:21Z

The variable may not work properly if you do find_package(CUDA) (which unfortunately is still necessary if you want CMake to create IDE projects).

Mhm? We are using project(... CXX CUDA) in our project and still have an IDE project. So I don't see any reason why someone should still use find_package(CUDA) after CMake 3.8.

claudiofantacci · 2019-06-07T11:40:08Z

Mhm? We are using project(... CXX CUDA) in our project and still have an IDE project. So I don't see any reason why someone should still use find_package(CUDA) after CMake 3.8.

Not all IDE generators support CUDA as a language. For example Xcode does not support it, MSVC does support it, and so forth. PCL project does not set CUDA as a language so you can generate an IDE project out of it for (theoretically) any IDE.

In the particular case of XCode, you can have a look at this issue on CMake's GitLab.

SunBlack · 2019-06-07T11:55:59Z

Good to know, seems we have luck, that we don't use XCode ;).

SergioRAgostinho · 2019-06-13T09:12:36Z

As of now, I see no way around other than adding the explicit flag to NVCC. As you can see, I've added directly the C++14 standard which from what I can tell was only added in CUDA 9.0. I haven't tried yet to see if thing compile simply with C++11. I just wanted to know what's the general opinion on deprecating everything below CUDA 9 if needed.

SunBlack · 2019-06-13T09:56:43Z

I just wanted to know what's the general opinion on deprecating everything below CUDA 9 if needed.

As far as I remember you have still Ubuntu 16.04 as lowest compile target and there is only CUDA 7.5 in default repository. So we have following choice:

Drop CUDA support below 9.0 => requires using repository of Nvidia on older Linux distributions (=> could affects your Azure builds).
Only allow C++11 in *.cu-files => CUDA 7.5 still supported

@ your latest changes: Does this works?

  if(CMAKE_VERSION VERSION_LESS 3.10)
    list(APPEND CUDA_NVCC_FLAGS "-D_FORCE_INLINES" $<$<OR:$<BOOL:CMAKE_COMPILER_IS_GNUXX>,$<BOOL:CMAKE_COMPILER_IS_CLANG>>:-std=c++14>)
  else()
    set(CMAKE_CUDA_STANDARD 14)
  endif()

Or does CMAKE_CUDA_STANDARD just work in case you are working with enable_language(CUDA)?

SergioRAgostinho · 2019-06-13T10:06:04Z

Apologies for the massive derailing this PR took. Quick update, just setting the flag to c++11 won't work. Some cuda code includes indirectly pcl_config.h and then

/Users/neglective/Development/3rdparty/pcl/build/include/pcl/pcl_config.h:7:4: error: PCL requires C++14 or above
  #error PCL requires C++14 or above

Or does CMAKE_CUDA_STANDARD just work in case you are working with enable_language(CUDA)?

No idea. I'll give it a try once I manage to compile all cuda code. At this moment only pcl_cuda_io is failing.

Edit:

Drop CUDA support below 9.0 => requires using repository of Nvidia on older Linux distributions (=> could affects your Azure builds).

NVidia has their own ppa which can be added to the current docker image.

SunBlack · 2019-06-13T13:01:11Z

Even I like 38a30fa - isn't it something for a separate PR, because it does not have anything to do with Clang?

SergioRAgostinho · 2019-06-13T13:30:46Z

Even I like 38a30fa - isn't it something for a separate PR, because it does not have anything to do with Clang?

Totally. I'll pull it out and open a separate PR. Don't be lazy like me.

SergioRAgostinho · 2019-07-18T20:29:22Z

Big request: in case any of you has some apps done with pcl_cuda code, could you test it against this PR. As you can see, host code is being compiled against c++14 and cuda code is being compiled with c++11. From I could tell, GCC should raise no issues as long as everything is compiled with the same compiler, or with a compiler version in which the C++11 standard features were already stable. I suspect the same will be applicable to clang. Running kinfu should (hopefully) be a good test to validate if things are alright.

In summary:

Compile pcl with everything related to cuda and gpu active, using this branch
Plug in an openni compatible depth camera, or something which works with the OpenNi viewer out of the box
Run pcl_kinfu.
If you have custom consumer code making use of classes in gpu or cuda, see if they run without issues.

Help \o/

claudiofantacci · 2019-07-19T07:13:55Z

1. Compile pcl with everything related to cuda and gpu active, using this branch

I can just do this as I don't have any code using pcl_cuda. Do you want me to give it a shot?

taketwo · 2019-07-19T08:31:29Z

I wanted to test this PR on Ubuntu 18.04. The first step was to test whether kinect_viewer_cuda works as is, without this change. Unfortunately, the app uses the old OpenNI grabber (not OpenNI2), and at least on my machine it fails to capture data from Asus Xtion. (The NiViewer app shipped with OpeNI also does not work.) So I tried to change kinect_viewer_cuda to use OpenNI2 grabber. It's not as trivial as adding 2 to the name, because it uses different image structures. And the methods in CUDA modules are not specialized for the new image structures. I gave a quick shot at updating those, but hit a problem similar to this, which can not be resolved without updating to new Eigen version.

Unfortunately I don't have much free time now, so have to get out of this rabbit hole until it's too late. Personally, I'd just avoid touching all this stuff. I never used it and don't know the status, but it seems to be totally outdated. Maybe we just set the flags as proposed such that the beast compiles, but add a warning "use at your own risk, might not work properly".

SunBlack · 2019-07-19T14:20:24Z

Just to remember as 3.15 is now official:

I don't know if PCL is supporting ARM, but CMake 3.15 will introduce ARMClang. Maybe we should add this, too.

Plug in an openni compatible depth camera, or something which works with the OpenNi viewer out of the box

I don't have any device on my PC like this, so I can't test it :(

SergioRAgostinho · 2019-07-19T19:36:54Z

I don't know if PCL is supporting ARM, but CMake 3.15 will introduce ARMClang. Maybe we should add this, too.

I incorporated a change that @Morwenn proposed in another PR. It should cover that case as well. See https://stackoverflow.com/questions/10046114/in-cmake-how-can-i-test-if-the-compiler-is-clang/10055571#10055571

I also haven't forgotten about this

 if(CMAKE_VERSION VERSION_LESS 3.10)
    list(APPEND CUDA_NVCC_FLAGS "-D_FORCE_INLINES" $<$<OR:$<BOOL:CMAKE_COMPILER_IS_GNUXX>,$<BOOL:CMAKE_COMPILER_IS_CLANG>>:-std=c++14>)
  else()
    set(CMAKE_CUDA_STANDARD 14)
  endif()

I will test a modified version of this, bearing in mind the info from @claudiofantacci that it isn't working with XCode.

SergioRAgostinho · 2019-07-19T20:27:20Z

I can just do this as I don't have any code using pcl_cuda. Do you want me to give it a shot?

I believe I have the same dev env as you currently, so I expect no issues.

$ clang --version
Apple LLVM version 9.0.0 (clang-900.0.39.2)
Target: x86_64-apple-darwin17.7.0
Thread model: posix
InstalledDir: /Library/Developer/CommandLineTools/usr/bin
$ nvcc --version
nvcc: NVIDIA (R) Cuda compiler driver
Copyright (c) 2005-2018 NVIDIA Corporation
Built on Tue_Jun_12_23:08:12_CDT_2018
Cuda compilation tools, release 9.2, V9.2.148

If something is different and/or you feel bored, by all means give it a go :)

stale · 2019-09-19T12:30:24Z

This pull request has been automatically marked as stale because it hasn't had
any activity in the past 60 days. Commenting or adding a new commit to the
pull request will revert this.

Come back whenever you have time. We look forward to your contribution.

kunaltyagi · 2020-04-28T10:39:59Z

Are you still working on this, or should I remove the milestone of 1.11 from this PR?

SergioRAgostinho · 2020-04-28T11:14:53Z

Remove from the milestone. I can no longer work on this because I decided to upgrade OS and Catalina still doesn't have CUDA support, nor I expect to to have it anytime soon.

stale · 2020-05-30T07:56:49Z

Marking this as stale due to 30 days of inactivity. Commenting or adding a new commit to the pull request will revert this.

themightyoarfish · 2022-01-27T10:54:33Z

This fixes #5120, would be great to have this merged, or use the fix in #5120.

SergioRAgostinho added the module: cmake label Jun 6, 2019

This comment has been minimized.

Sign in to view

SergioRAgostinho added the platform: macos label Jun 7, 2019

This was referenced Jun 13, 2019

Suppress deprecation warnings in gpu. #3153

Merged

Use COMPONENTS when finding VTK to avoid linking agains unnecessary modules #3140

Merged

SergioRAgostinho mentioned this pull request Jul 5, 2019

[gpu] Prefer lambdas over binds. #3209

Merged

SergioRAgostinho mentioned this pull request Jul 17, 2019

Support AppleClang in CMakeLists.txt #3242

Closed

SergioRAgostinho force-pushed the apple-clang branch 2 times, most recently from 0e03f3f to 3355fff Compare July 17, 2019 08:27

Identify AppleClang as a valid Clan compiler

2e489a2

Explicitly add c++11 flag to nvcc on unix platforms.

16054ac

SergioRAgostinho force-pushed the apple-clang branch from 3355fff to 16054ac Compare July 18, 2019 18:12

SergioRAgostinho added this to the pcl-1.10.0 milestone Jul 21, 2019

stale bot added the status: stale label Sep 19, 2019

kunaltyagi modified the milestones: pcl-1.10.0, pcl-1.11.0 Feb 21, 2020

stale bot removed the status: stale label Feb 21, 2020

kunaltyagi removed this from the pcl-1.11.0 milestone Apr 28, 2020

stale bot added the status: stale label May 30, 2020

mvieth mentioned this pull request Jan 5, 2022

[cmake] AVX not detected on macos #5120

Closed

stale bot removed the status: stale label Jan 27, 2022

mvieth mentioned this pull request Feb 2, 2022

Treat all compiler IDs containing 'Clang' as clang #5153

Merged

mvieth closed this in #5153 Feb 5, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Identify AppleClang as a valid Clang compiler #3131

Identify AppleClang as a valid Clang compiler #3131

SergioRAgostinho commented Jun 6, 2019

SunBlack commented Jun 6, 2019 •

edited

Loading

SergioRAgostinho commented Jun 6, 2019

taketwo commented Jun 6, 2019

SergioRAgostinho commented Jun 6, 2019 •

edited

Loading

taketwo commented Jun 6, 2019

SergioRAgostinho commented Jun 6, 2019 •

edited

Loading

claudiofantacci commented Jun 6, 2019

This comment has been minimized.

SergioRAgostinho commented Jun 7, 2019 •

edited

Loading

claudiofantacci commented Jun 7, 2019 •

edited

Loading

SunBlack commented Jun 7, 2019

claudiofantacci commented Jun 7, 2019

claudiofantacci commented Jun 7, 2019

SunBlack commented Jun 7, 2019

claudiofantacci commented Jun 7, 2019 •

edited

Loading

SunBlack commented Jun 7, 2019

SergioRAgostinho commented Jun 13, 2019

SunBlack commented Jun 13, 2019 •

edited by SergioRAgostinho

Loading

SergioRAgostinho commented Jun 13, 2019 •

edited

Loading

SunBlack commented Jun 13, 2019

SergioRAgostinho commented Jun 13, 2019

SergioRAgostinho commented Jul 18, 2019

claudiofantacci commented Jul 19, 2019

taketwo commented Jul 19, 2019

SunBlack commented Jul 19, 2019

SergioRAgostinho commented Jul 19, 2019

SergioRAgostinho commented Jul 19, 2019

stale bot commented Sep 19, 2019

kunaltyagi commented Apr 28, 2020

SergioRAgostinho commented Apr 28, 2020

stale bot commented May 30, 2020

themightyoarfish commented Jan 27, 2022

Identify AppleClang as a valid Clang compiler #3131

Identify AppleClang as a valid Clang compiler #3131

Conversation

SergioRAgostinho commented Jun 6, 2019

SunBlack commented Jun 6, 2019 • edited Loading

SergioRAgostinho commented Jun 6, 2019

taketwo commented Jun 6, 2019

SergioRAgostinho commented Jun 6, 2019 • edited Loading

taketwo commented Jun 6, 2019

SergioRAgostinho commented Jun 6, 2019 • edited Loading

claudiofantacci commented Jun 6, 2019

This comment has been minimized.

SergioRAgostinho commented Jun 7, 2019 • edited Loading

claudiofantacci commented Jun 7, 2019 • edited Loading

SunBlack commented Jun 7, 2019

claudiofantacci commented Jun 7, 2019

claudiofantacci commented Jun 7, 2019

SunBlack commented Jun 7, 2019

claudiofantacci commented Jun 7, 2019 • edited Loading

SunBlack commented Jun 7, 2019

SergioRAgostinho commented Jun 13, 2019

SunBlack commented Jun 13, 2019 • edited by SergioRAgostinho Loading

SergioRAgostinho commented Jun 13, 2019 • edited Loading

SunBlack commented Jun 13, 2019

SergioRAgostinho commented Jun 13, 2019

SergioRAgostinho commented Jul 18, 2019

claudiofantacci commented Jul 19, 2019

taketwo commented Jul 19, 2019

SunBlack commented Jul 19, 2019

SergioRAgostinho commented Jul 19, 2019

SergioRAgostinho commented Jul 19, 2019

stale bot commented Sep 19, 2019

kunaltyagi commented Apr 28, 2020

SergioRAgostinho commented Apr 28, 2020

stale bot commented May 30, 2020

themightyoarfish commented Jan 27, 2022

SunBlack commented Jun 6, 2019 •

edited

Loading

SergioRAgostinho commented Jun 6, 2019 •

edited

Loading

SergioRAgostinho commented Jun 6, 2019 •

edited

Loading

SergioRAgostinho commented Jun 7, 2019 •

edited

Loading

claudiofantacci commented Jun 7, 2019 •

edited

Loading

claudiofantacci commented Jun 7, 2019 •

edited

Loading

SunBlack commented Jun 13, 2019 •

edited by SergioRAgostinho

Loading

SergioRAgostinho commented Jun 13, 2019 •

edited

Loading