[GitHub] KellenSunderland edited a comment on issue #12160: Remove conflicting llvm OpenMP from cmake builds

[GitBox <gi...@apache.org>]

KellenSunderland edited a comment on issue #12160: Remove conflicting llvm OpenMP from cmake builds
URL: https://github.com/apache/incubator-mxnet/pull/12160#issuecomment-415039728
 
 
   LGTM, might not work on older versions of MacOS as mentioned, but I think this is partially ok given it's broken already and that soon the current release, and current release -1 should both support openmp from clang.
   
   Orthogonal to this PR, but @szha @lebeg: Wouldn't we expect the CMake behaviour would detect if you have OpenMP (or don't have it) and take appropriate actions without an error?  Why do we even need to specify OPEN_MP=0/1?  If we were setting this automatically we would not run into different versions of MacOS building/failing right?  What do you think?

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services