Closed klimkin closed 3 years ago
I see a recent commit adding -gdwarf-3. Please see if it's still needed. If so, will add to some of CMAKE_CXXFLAGS* variables.
I believe the answer to this question is "yes". Let me know when you think this is safe to merge.
Both benchmarks and the library?
On Sat, Jan 9, 2021, 1:19 PM Emery Berger notifications@github.com wrote:
I see a recent commit adding -gdwarf-3. Please see if it's still needed. If so, will add to some of CMAKE_CXXFLAGS* variables.
I believe the answer to this question is "yes". Let me know when you think this is safe to merge.
— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/plasma-umass/coz/pull/178#issuecomment-757368857, or unsubscribe https://github.com/notifications/unsubscribe-auth/AADD3XPSTKLIPB3JNRLBSIDSZDB53ANCNFSM4V2W27BA .
Probably both just to be on the safe side. See https://github.com/plasma-umass/coz/issues/107#issuecomment-708341054.
I quickly glanced over the changes. As someone with a fair bit of experience with CMake I found a few things that could be changed to improve maintainability, developer experience and integration/consumption in other projects. If you're interested in that input I can compile a list or add comments in the diff view.
Of course, please do!
On Sat, Jan 9, 2021, 3:03 PM Manuel Bergler notifications@github.com wrote:
I quickly glanced over the changes. As someone with a fair bit of experience with CMake I found a few things that could be changed to improve maintainability, developer experience and integration/consumption in other projects. If you're interested in that input I can compile a list or add comments in the diff view.
— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/plasma-umass/coz/pull/178#issuecomment-757380718, or unsubscribe https://github.com/notifications/unsubscribe-auth/AADD3XJ7IJSWP63QSVDUUADSZDOETANCNFSM4V2W27BA .
Hi @klimkin and @Corristo - let me know if there’s anything I can do on this front. Thanks!
@Corristo It's ready for the next iteration of the review. Please see my comments above.
Hi @klimkin and @Corristo - let me know if there’s anything I can do on this front. Thanks!
@emeryberger Could you see if it builds and works the same way on your system, please?
Merge?
Some differences from original build - might need adjustments, please test on your platform:
-DCMAKE_BUILD_TYPE=RelWithDebInfo
, this way everything builds with "-g -O2".-gdwarf-3
. Please see if it's still needed. If so, will add to some ofCMAKE_CXX_FLAGS_*
variables.-DBUILD_BENCHMARKS=ON
cmake option.conanfile.txt
). Appears to be most portable option.Full list of benchmark run targets: