dmlc / dmlc-core

A common bricks library for building scalable and portable distributed machine learning.
Apache License 2.0
864 stars 520 forks source link

Fix include/dmlc/omp.h for old Android versions #585

Closed leezu closed 4 years ago

leezu commented 4 years ago

https://github.com/dmlc/dmlc-core/pull/582#issuecomment-564334186

hcho3 commented 4 years ago

@larroy Can you review?

larroy commented 4 years ago

I'm not 100% sure about the logic from the top of my head. Have you guys tried with the MXNet build which uses the NDK? CI is running android builds in mxnet.

leezu commented 4 years ago

__GOMP_NOTHROW is GNU OpenMP (GCC) specific. So the __ANDROID__ part of the condition doesn't make sense as you pointed out in https://github.com/dmlc/dmlc-core/pull/582#issuecomment-564334186.

larroy commented 4 years ago

Sorry I don't remember this, it's swapped out. If MXNet passes CI with this change I'm fine with it.

leezu commented 4 years ago

Tested by https://github.com/apache/incubator-mxnet/pull/17095

larroy commented 4 years ago

Then lgtm

leezu commented 4 years ago

@hcho3 please help to merge.