Project

General

Profile

Task #1855

Convert preprocessor use so that symbols are always defined

Added by Mark Abraham about 4 years ago. Updated over 3 years ago.

Status:
New
Priority:
Normal
Assignee:
-
Category:
-
Target version:
-
Difficulty:
uncategorized
Close

Description

We will end up with no preprocessor constructs that check whether GROMACS preprocessor symbols are defined, because we will always define them to a value and check that instead. This is much more robust under maintenance, and is something other large projects have done also (e.g. https://sourceware.org/glibc/wiki/Wundef). We've had several issue arise through not doing this as well as we could (e.g. #1673). Once complete, we can use gcc -Wundef to keep the code like that, and have fewer home-grown scripts checking things.

So far, the SIMD module is completed, but src/config.h.in needs a lot more #cmakedefine01 (and associated code fixes).


Subtasks

Bug #1673: mis-use of simd.h after refactoring ClosedMark Abraham

Associated revisions

Revision 3451072f (diff)
Added by Mark Abraham almost 4 years ago

Update use of preprocessor in managing GPU support

Once CMake has done detection of GPU library support, within CMake we
now use GMX_USE_CUDA and GMX_USE_OPENCL to handle details such as
which source code files should be compiled. This makes code in
CMakeLists.txt files slightly more clear.

To configure the GROMACS build via config.h, CMake sets GMX_GPU_TYPE
to match the name of one of three hard-coded defines in
config.h.cmake.in, and configures GMX_GPU with the value represented
by that name. This ensures GMX_GPU always has a value in source code,
so various kinds of mis-use will found by the compiler. It also means
we can use GMX_GPU and the values of those defines to simplify the
parts of high-level code that are different according to which GPU
configuration is in use - mostly in reporting to the user what is
going on. This reduces the number of complex preprocessor conditionals
that might want documenting or indenting, and makes it harder to write
a syntax error that can only be found with a particular build
configuration.

Minor change to the start-of-run reporting. Rather than show that GPU
is disabled/enabled, and OpenCL likewise, show disabled or which of
CUDA or OpenCL is enabled for GPU acceleration. The OpenCL library
name will make clear whether AMD or NVIDIA libraries is providing the
OpenCL runtime.

No changes to user interface of CMake. Removed redundant declaration
of option(GMX_GPU) from main CMakeLists.txt, since gmxManageGpu()
already did that.

No changes required to current or future compute code, since CMake
still handles whether such code is compiled at all.

Refs #1855

Change-Id: I3448fe284ac526eb2b185e915b95fcc84f3d469a

Revision 8c713819 (diff)
Added by Mark Abraham almost 4 years ago

Change GMX_DOUBLE to be always defined

Also documented use of precision-related CMake variables

There may be some references in the SIMD documentation that still
refer to GMX_DOUBLE being defined, but we should solve that
separately, given the other changes at the moment.

Some abuse that un-defined GMX_DOUBLE was hard-coding some use of
single precision in mdebin_bar.cpp, so made that explicit.

Removed GMX_CPT_BUILD_DP, which can now be replaced by GMX_DOUBLE.

Removed some #ifdef in QM code, because floating-point arguments to
printf are automatically promoted to double. scanf still needs
versioning, though.

Refs #1855

Change-Id: I59b6874739d528603a86e0b6ab9bcadd67fdd3fa

Revision 533651b1 (diff)
Added by Szilárd Páll almost 4 years ago

remove duplicate macros and enable Wundef in CUDA

Commit adbada4 left a checks of the host-side undefined CUDA_ARCH
undefined macro behind in nbnxn_cuda.cu as well as some unused macros.
This change cleans up these leftovers.

Additionally, this change enables -Wundef for CUDA files, but only for
CUDA >=v7.5 as prior versions come with a header that uses a macro check
without checking.

As the host-side code still includes kernels with CUDA_ARCH which is
not defined in the host compilation pass, we need create our own copy
of this arch macro in a new header that is meant to contain CUDA
arch-specific stuff.

Refs #1855

Change-Id: Ibab891eaee11ea8952a67125f7ac4cc620b88d1c

History

#1 Updated by Gerrit Code Review Bot about 4 years ago

Gerrit received a related patchset '2' for Issue #1855.
Uploader: Mark Abraham ()
Change-Id: I59b6874739d528603a86e0b6ab9bcadd67fdd3fa
Gerrit URL: https://gerrit.gromacs.org/5343

#2 Updated by Gerrit Code Review Bot about 4 years ago

Gerrit received a related patchset '2' for Issue #1855.
Uploader: Mark Abraham ()
Change-Id: I3448fe284ac526eb2b185e915b95fcc84f3d469a
Gerrit URL: https://gerrit.gromacs.org/5360

#3 Updated by Gerrit Code Review Bot almost 4 years ago

Gerrit received a related patchset '6' for Issue #1855.
Uploader: Szilárd Páll ()
Change-Id: Ibab891eaee11ea8952a67125f7ac4cc620b88d1c
Gerrit URL: https://gerrit.gromacs.org/5107

#4 Updated by Teemu Murtola almost 4 years ago

With https://gerrit.gromacs.org/5546 (and its parent), all of config.h now uses 0/1 macros, except for macros that are also used by thread-MPI and would thus require adaptation there as well.

#5 Updated by Mark Abraham over 3 years ago

Some uses of plain #cmakedefine have snuck in, probably from merges release-5-1, so we should go through and clean up. Not sure whether best in release-2016 or master.

Also available in: Atom PDF