Honor `CXX_FLAGS` of the system

douban / paracel

Distributed training framework with parameter server

http://paracel.io

Other

339 stars 84 forks source link

Honor `CXX_FLAGS` of the system #18

Closed windreamer closed 8 years ago

windreamer commented 8 years ago

and install headers

windreamer commented 8 years ago

-mtune=native makes the binary generated not portable enough for different kinds of CPUs

xunzhang commented 8 years ago

It is cool modification for default gcc compiler which supports c++11. There are two questions:

I think it is a trade-off between the deployment effort with the running performance. If machine compiles itself instead of copying the binary package from others, it makes sense using this option?
Why this pull request removes the part of switching different compile mode?

ps: I am not sure now who is the maintainer of libdoubanm, doubanm, and some related c++ projects. I think all this projects might have similar problems.

windreamer commented 8 years ago

for most system, packages are delivered in binary. and the author of the package should do the trade-off of the performance, so -mtune=native is not ideal fixed in CMakeLIsts. End-user can specify his own compiler arguments to better trade-off according his environment, by add -DCMAKE_CXX_FLAGS=...
I think you refer the CMAKE_BUILD_TYPE part? CMAKE_BUILD_TYPE can be specified by -DCMAKE_BUILD_TYPE, I think we can add some build rules for convenience, eg, debug_rules.cmake or release_rules.cmake

xunzhang commented 8 years ago

Ok, agree! Could you add, say debug_rules.cmake or release_rules.cmake inside this pull request to make the refactor logic more clearly?

xunzhang commented 8 years ago

LGTM 🍺