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

Simplify makefiles by guarding compilation with appropriate KOKKOS_ENABLE_### macros #716

Closed
dsunder opened this issue Mar 30, 2017 · 6 comments
Labels
Enhancement Improve existing capability; will potentially require voting
Milestone

Comments

@dsunder
Copy link
Contributor

dsunder commented Mar 30, 2017

Remove conditional logic in Makefiles by protecting files with appropriate KOKKOS_ENABLE_### macros. Also prevent linker from warning about empty archive. Only Kokkos_Macros.hpp should be included before the #if defined test.

eg. (Kokkos_Foo.cpp)

#include <Kokkos_Macros.hpp>
#if defined( KOKKOS_ENABLE_FOO )
...
#else
int g_KOKKOS_FOO_PREVENT_EMPTY_LINK_ERROR ;
#endif //if defined(KOKKOS_ENABLE_FOO)

@gmackey
Copy link
Contributor

gmackey commented Mar 30, 2017

@dsunder I've already got updates to suppress the linker errors in my cmake branch. It does it from the makefile side by conditional includes. Why is it better to not have the conditional files but compile almost empty ones? You then obviously have a few extra files in the compile that aren't even needed. I would argue excluding them based on conditionals in the Makefile is better.

@hcedwar
Copy link
Contributor

hcedwar commented Mar 30, 2017

Better to define a noop function than global variable.
void KOKKOS_FOO_NOOP_TO_AVOID_EMPTY_OBJECT_FILE() {}

@hcedwar
Copy link
Contributor

hcedwar commented Mar 30, 2017

Which strategy has better maintainability? make and cmake file logic or macro logic?
How does the macro strategy with extra compiles of trivial files impact compile times?

@dsunder
Copy link
Contributor Author

dsunder commented Mar 30, 2017

I think that adding the protection to the files is valuable regardless of what the build system does. The build system is free to exclude the files and it doesn't cost us anything, but if the build includes extra files it will still work without warnings.

@hcedwar hcedwar added the Enhancement Improve existing capability; will potentially require voting label Apr 5, 2017
@hcedwar
Copy link
Contributor

hcedwar commented Apr 5, 2017

This strategy has lower maintenance burden.

@hcedwar hcedwar added this to the 2017-April-end milestone Apr 5, 2017
@crtrott crtrott modified the milestones: 2017-April-end, 2017-June-end Apr 26, 2017
@ibaned
Copy link
Contributor

ibaned commented Jun 14, 2017

This is all the way in master by now, we missed it.

@ibaned ibaned closed this as completed Jun 14, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement Improve existing capability; will potentially require voting
Projects
None yet
Development

No branches or pull requests

5 participants