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

tbb 4.4.4 does not seem to handle clang. #669

Closed
fulara opened this issue Mar 1, 2019 · 8 comments
Closed

tbb 4.4.4 does not seem to handle clang. #669

fulara opened this issue Mar 1, 2019 · 8 comments

Comments

@fulara
Copy link

fulara commented Mar 1, 2019

Package and Environment Details (include every applicable attribute)

  • Package Name/Version: tbb/4.4.4
  • Operating System+version: Linux Ubuntu 16.04
  • Compiler+version: clang-7.0
  • Docker image: lasote/conanclang60
  • Conan version: conan 1.12.3
  • Python version: Python 3.6.0

Short description

I tried to build tbb version 4.x but it seemed that tbb was being built with gcc instead. I looked briefly at tbb recipes and they do mention clang so I guess its supported.

What did I do?

I forked off from your stable/4.4.4 and modified travis.yml to be like this:
env: CONAN_CLANG_VERSIONS=7.0 CONAN_DOCKER_IMAGE=lasote/conanclang60
can be seen here: https://github.com/fulara/conan-tbb/blob/stable/4.4.4/.travis.yml

What did I expect?

That tbb would be built with clang

what did i get?

In the build logs that are pasted below i can see following (examplary line)
g++ -o concurrent_vector.o -c -MMD -DTBB_USE_DEBUG -DDO_ITT_NOTIFY -g -O0 -DUSE_PTHREAD -m32 -march=pentium4 -mrtm -fPIC -D__TBB_BUILD=1 -Wall -Wno-parentheses -Wno-non-virtual-dtor -I../../src -I../../src/rml/include -I../../include ../../src/tbb/concurrent_vector.cpp
So it seems to me that TBB is being built with g++ rather than clang here.
Apologies if i made a mistake during setup of this travil.yml, however i saw this issue at work where i tried using:
conan ... -s "compiler=clang" -s "compiler.version=7.0" and i got similar output as in attached travis output

Steps to reproduce (Include if Applicable)

Logs (Include/Attach if Applicable)

https://travis-ci.org/fulara/conan-tbb/builds/500563739
https://github.com/fulara/conan-tbb/blob/stable/4.4.4/.travis.yml

@fulara fulara added the bug label Mar 1, 2019
@madebr
Copy link
Contributor

madebr commented Mar 2, 2019

I'm just wondering.
Is there a reason why you're not using tbb/2019_U3@conan/stable?
The 4.4 branch on the official tbb github repo is 3 years old.

You should add clang7 packages to your travis.yml as:

      - <<: *linux
        env: CONAN_CLANG_VERSIONS=7.0 CONAN_DOCKER_IMAGE=conanio/clang7

@fulara
Copy link
Author

fulara commented Mar 2, 2019

Updated my travis to use that docker image, g++ still being invoked. travis log
as to why i am using tbb thats 3 yo.. because i started using it ~3 years ago. :) I noticed that it seems that clang build works fine for that 2019_U3, so i am actually considering upgrading to that latest tbb.

Still, raised an issue because probably this is not expected behavior, I will leave this up to maintainers whether they want to take a look at that or not.

@SSE4
Copy link
Member

SSE4 commented Mar 4, 2019

@fulara we have merged required changes, please give an another try

@SSE4 SSE4 added fixed feedback Waiting for feedback from the original issue author. Gets closed after several weeks if no feedback labels Mar 4, 2019
@fulara
Copy link
Author

fulara commented Mar 4, 2019

@SSE4 will give it a go later on today.
by the way i wanted to upgrade to latest recipe but i noticed 2 things:
Please tell me if you want me to raise issue for below.

  1. in new recipes you are using:
    default_options = {"shared": False, "tbbmalloc": False, "tbbproxy": False}
    shared defaults to 'False' whereas in old version it was shared: True. intended?
  2. in new recipes i can see this, looks like a wrong if conditoin to me:
if self.settings.os != "Windows" and self.options.shared:
            self.output.warn("Intel-TBB strongly discourages usage of static linkage")

Isnt it meant to be
if self.settings.os != "Windows" and not self.options.shared ?

@no-response no-response bot removed the feedback Waiting for feedback from the original issue author. Gets closed after several weeks if no feedback label Mar 4, 2019
@madebr
Copy link
Contributor

madebr commented Mar 4, 2019

@fulara This change to shared=False happened in conan-community/conan-tbb@966e493 by @uilianries .
That same commit also introduced the bad test for warning about a static tbb build.

I think it is a bug.

@SSE4
Copy link
Member

SSE4 commented Mar 4, 2019

shared cannot be default, as Windows doesn't support shared libraries, so it will end up into non-working default builds.
also, it's general practice for the most of conan packages to use shared=False by default, due to the various limitation of shared builds.
you still may apply tbb:shared=True in your profile or on command line, so it's just matter of default preferences.

@fulara
Copy link
Author

fulara commented Mar 4, 2019

@SSE4 Yep, this is something I expected - so to my opinion just that if statement is confusing.

I tested 4.4.4 and its working fine now. i did notice although a weird thing with 'version' not being propagated, but thats only used for building so its not a huge issue.

Closing this issue.

@fulara fulara closed this as completed Mar 4, 2019
@kepam
Copy link

kepam commented Mar 5, 2019

Still this warning

if self.settings.os != "Windows" and self.options.shared:
            self.output.warn("Intel-TBB strongly discourages usage of static linkage")

looks as mistake
could you @SSE4 , comment on this?

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

No branches or pull requests

4 participants