Skip to content

Conversation

@xvallspl
Copy link
Contributor

This way when we introduce dependencies in TMath that require linkage (f.e. Vc) it won't suppose a problem for libCore.

@phsft-bot
Copy link

Starting build on centos7/gcc49, mac1011/native, slc6/gcc49, slc6/gcc62, ubuntu14/native

@xvallspl xvallspl force-pushed the timestamp branch 3 times, most recently from 9474b9e to 080e804 Compare March 29, 2017 13:23
@vgvassilev
Copy link
Member

Hm, could you clarify a little more?

@phsft-bot
Copy link

Starting build on centos7/gcc49, mac1011/native, slc6/gcc49, slc6/gcc62, ubuntu14/native

2 similar comments
@phsft-bot
Copy link

Starting build on centos7/gcc49, mac1011/native, slc6/gcc49, slc6/gcc62, ubuntu14/native

@phsft-bot
Copy link

Starting build on centos7/gcc49, mac1011/native, slc6/gcc49, slc6/gcc62, ubuntu14/native

@xvallspl
Copy link
Contributor Author

xvallspl commented Mar 29, 2017

Hi, Vassil.

Having Vc types in TMath required me to link TTimeStamp.cxx with -lVc, and TTimeStamp is in Core (doesn't need to link with Vc at the moment)

@phsft-bot
Copy link

Starting build on centos7/gcc49, mac1011/native, slc6/gcc49, slc6/gcc62, ubuntu14/native

@xvallspl
Copy link
Contributor Author

xvallspl commented Mar 29, 2017

Btw, what's with all these builds starting so many times?

@vgvassilev
Copy link
Member

I will investigate this tomorrow. It seems a bug if we depend on Vc types there.

@dpiparo
Copy link
Member

dpiparo commented Mar 30, 2017

Hi,
I think using mathematical functions coming from libm without wrapping them within the ROOT ones is ok. We do the same in PROOF: to get random socket numbers we leverage the STL random number generator rather than the Math one to avoid dependencies.

@dpiparo dpiparo merged commit f32175e into root-project:master Mar 30, 2017
@vgvassilev
Copy link
Member

I am confused. I don't see how TMath.h depends on Vc and why you need to link against Vc.

@dpiparo, one disadvantage of using functions from STL by qualified names is Vc. This doesn't allow users that include Vc (or any other vectorization library using the ADL pattern) to vectorize their code.

I do not like duplicating constants lib Pi all over the place. Let's discuss that in person.

@xvallspl
Copy link
Contributor Author

It doesn't yet. It's a problem we encountered when GSoC candidates started working on vectorizing functions in TMath.

@xvallspl
Copy link
Contributor Author

Which in the end can be solved having the vectorized implementations in a separate header file, but that may mean replicating code, so... whatever you think is best.

There's actually another file with the same problem in io/io: TFileCacheRead.cxx

@vgvassilev
Copy link
Member

I think this is a layering violation in general. I am in favor of breaking it (Core shouldn't need things from Math). However, we should solve the problem with duplicating code because we want to disentangle them. The Pi constant (among with others in Math, I assume) which is needed in Core should go in some form in code (like other constants in RtypesCore.h).

@pcanal
Copy link
Member

pcanal commented Mar 30, 2017

Hi,

There is already a TMathBase.h that could be used for that. That said. It might make sense to keep TMath.h as is and to introduce TMathVec.h which has the extra dependency and is use in code that expects/wants to support vector code.

Cheers,
Philippe.

@vgvassilev
Copy link
Member

So let's move at least the Pi constant in TMathBase.h. I feel that introducing TMathVec.h would be reasonable if we have blockers in TMath.h. Otherwise it would seem that we diverge for no reason.

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

Successfully merging this pull request may close these issues.

5 participants