Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

OC_OMP support is incorrect. #218

Closed
linas opened this issue Dec 31, 2019 · 2 comments
Closed

OC_OMP support is incorrect. #218

linas opened this issue Dec 31, 2019 · 2 comments

Comments

@linas
Copy link
Member

linas commented Dec 31, 2019

Follow-on to pull-req #217

So: FIND_PACKAGE(ParallelSTL) works as designed, and cmake sets it correctly:

FIND_PACKAGE(ParallelSTL)
IF (PARALLEL_STL_FOUND)
   ADD_DEFINITIONS(-DHAVE_PARALLEL_STL)
ENDIF (PARALLEL_STL_FOUND)

However, the users of oc_omp.h (or at least, this user) do not have a -DHAVE_PARALLEL_STL in their code, nor do they have any #define OC_OMP either. As a result, those wonderful parallel loops are not actually .. parallel.

Moving to c++17 per #177 seems a little more important, now.

linas added a commit to linas/atomspace that referenced this issue Dec 31, 2019
@ngeiswei
Copy link
Member

ngeiswei commented Jan 8, 2020

I think this can be closed, reopen otherwise.

@ngeiswei ngeiswei closed this as completed Jan 8, 2020
@linas
Copy link
Member Author

linas commented Jan 8, 2020

Well, I don't know if this should be re-opened or not ... because I can't tell if its a bug or a feature. Basically, OC_OMP didn't do what I thought it did, and this was a surprise. My plan is to avoid it in new code. So maybe just ignoring this is best.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants