Minor change to GPU compatibility reporting handling

The old code returned an int that was used to look up an
array of strings, and analysis programs can't tell whether
the use was valid.

While we could try to return an enum, instead the logic
of whether a device is compatible is separated from
describing why it isn't.

Noted TODO to move the definition of those strings to a good single
place, when there is one.

Change-Id: I504f4e7efcfcb76f97873ff1f98bee9c67b157a3

diff --git a/src/gromacs/gpu_utils/gpu_utils.cu b/src/gromacs/gpu_utils/gpu_utils.cu
index 4027f574c7e68191a3d948bdc0c9f80463c67acf..3c9725a6068ebc1effce481f687b39d87211c982 100644 (file)
--- a/src/gromacs/gpu_utils/gpu_utils.cu
+++ b/src/gromacs/gpu_utils/gpu_utils.cu
@@ -626,12 +626,24 @@ int detect_gpus(gmx_gpu_info_t *gpu_info, char *err_str)
     return retval;
 }
 
-int getGpuCompatibilityStatus(const gmx_gpu_info_t *gpu_info,
-                              int                   index)
+bool isGpuCompatible(const gmx_gpu_info_t *gpu_info,
+                     int                   index)
 {
     assert(gpu_info);
 
-    return (index >= gpu_info->n_dev) ? egpuNonexistent : gpu_info->gpu_dev[index].stat;
+    return (index >= gpu_info->n_dev ?
+            false :
+            gpu_info->gpu_dev[index].stat == egpuCompatible);
+}
+
+const char *getGpuCompatibilityDescription(const gmx_gpu_info_t *gpu_info,
+                                           int                   index)
+{
+    assert(gpu_info);
+
+    return (index >= gpu_info->n_dev ?
+            gpu_detect_res_str[egpuNonexistent] :
+            gpu_detect_res_str[gpu_info->gpu_dev[index].stat]);
 }
 
 void free_gpu_info(const gmx_gpu_info_t *gpu_info)

diff --git a/src/gromacs/gpu_utils/gpu_utils.h b/src/gromacs/gpu_utils/gpu_utils.h
index 40c091e1208c83f52d50c16499cb55b4714a3f1d..86001cbdf1dfca93ae851bcf9d352b493dd19bc2 100644 (file)
--- a/src/gromacs/gpu_utils/gpu_utils.h
+++ b/src/gromacs/gpu_utils/gpu_utils.h
@@ -74,15 +74,25 @@ class MDLogger;
 GPU_FUNC_QUALIFIER
 int detect_gpus(struct gmx_gpu_info_t *GPU_FUNC_ARGUMENT(gpu_info), char *GPU_FUNC_ARGUMENT(err_str)) GPU_FUNC_TERM_WITH_RETURN(-1)
 
-/*! \brief Return compatibility status of GPU with given \c index.
+/*! \brief Return whether the GPU with given \c index is compatible, ie suitable for use.
  *
  * \param[in]   gpu_info    pointer to structure holding GPU information
  * \param[in]   index       index of GPU to ask about
- * \returns                 A e_gpu_detect_res_t value describing the status
+ * \returns                 Whether the GPU is compatible.
  */
 GPU_FUNC_QUALIFIER
-int getGpuCompatibilityStatus(const gmx_gpu_info_t *GPU_FUNC_ARGUMENT(gpu_info),
-                              int GPU_FUNC_ARGUMENT(index)) GPU_FUNC_TERM_WITH_RETURN(-1)
+bool isGpuCompatible(const gmx_gpu_info_t *GPU_FUNC_ARGUMENT(gpu_info),
+                     int GPU_FUNC_ARGUMENT(index)) GPU_FUNC_TERM_WITH_RETURN(false)
+
+/*! \brief Return a string describing how compatible the GPU with given \c index is.
+ *
+ * \param[in]   gpu_info    pointer to structure holding GPU information
+ * \param[in]   index       index of GPU to ask about
+ * \returns                 A null-terminated C string describing the compatibility status, useful for error messages.
+ */
+GPU_FUNC_QUALIFIER
+const char *getGpuCompatibilityDescription(const gmx_gpu_info_t *GPU_FUNC_ARGUMENT(gpu_info),
+                                           int GPU_FUNC_ARGUMENT(index)) GPU_FUNC_TERM_WITH_RETURN("")
 
 /*! \brief Frees the gpu_dev and dev_use array fields of \p gpu_info.
  *

diff --git a/src/gromacs/gpu_utils/gpu_utils_ocl.cpp b/src/gromacs/gpu_utils/gpu_utils_ocl.cpp
index 37ee78430a3e411c7754ff93230abefad32c9cb3..58296703a16d4da4cd015de5534cd8fedc936498 100644 (file)
--- a/src/gromacs/gpu_utils/gpu_utils_ocl.cpp
+++ b/src/gromacs/gpu_utils/gpu_utils_ocl.cpp
@@ -343,12 +343,25 @@ void free_gpu_info(const gmx_gpu_info_t gmx_unused *gpu_info)
 }
 
 //! This function is documented in the header file
-int getGpuCompatibilityStatus(const gmx_gpu_info_t *gpu_info,
-                              int                   index)
+bool isGpuCompatible(const gmx_gpu_info_t *gpu_info,
+                     int                   index)
 {
     assert(gpu_info);
 
-    return (index >= gpu_info->n_dev) ? egpuNonexistent : gpu_info->gpu_dev[index].stat;
+    return (index >= gpu_info->n_dev ?
+            false :
+            gpu_info->gpu_dev[index].stat == egpuCompatible);
+}
+
+//! This function is documented in the header file
+const char *getGpuCompatibilityDescription(const gmx_gpu_info_t *gpu_info,
+                                           int                   index)
+{
+    assert(gpu_info);
+
+    return (index >= gpu_info->n_dev ?
+            gpu_detect_res_str[egpuNonexistent] :
+            gpu_detect_res_str[gpu_info->gpu_dev[index].stat]);
 }
 
 //! This function is documented in the header file

diff --git a/src/gromacs/hardware/detecthardware.cpp b/src/gromacs/hardware/detecthardware.cpp
index 9a6c7b49c616c5161703d0f8fa3386dab5768737..0b627a36a61532ed5312c44eafa29a7d7ced7a44 100644 (file)
--- a/src/gromacs/hardware/detecthardware.cpp
+++ b/src/gromacs/hardware/detecthardware.cpp
@@ -121,6 +121,8 @@ static const bool multiGpuSupport[] = {
 };
 static const bool bMultiGpuPerNodeSupported = multiGpuSupport[GMX_GPU];
 
+// TODO If/when we unify CUDA and OpenCL support code, this should
+// move to a single place in gpu_utils.
 /* Names of the GPU detection/check results (see e_gpu_detect_res_t in hw_info.h). */
 const char * const gpu_detect_res_str[egpuNR] =
 {

diff --git a/src/gromacs/hardware/hardwareassign.cpp b/src/gromacs/hardware/hardwareassign.cpp
index 11a93668c158319d345eefc34e83a8e83fa918e8..224864e16b479b6b0a40b1de806918b6d9db5fde 100644 (file)
--- a/src/gromacs/hardware/hardwareassign.cpp
+++ b/src/gromacs/hardware/hardwareassign.cpp
@@ -189,13 +189,12 @@ static bool checkGpuSelection(const gmx_gpu_info_t *gpu_info,
         GMX_ASSERT(gpu_opt->dev_use, "Invalid gpu_opt->dev_use");
 
         int id     = gpu_opt->dev_use[i];
-        int status = getGpuCompatibilityStatus(gpu_info, id);
-        if (status != egpuCompatible)
+        if (!isGpuCompatible(gpu_info, id))
         {
             allOK    = false;
             message += gmx::formatString("    GPU #%d: %s\n",
                                          id,
-                                         gpu_detect_res_str[status]);
+                                         getGpuCompatibilityDescription(gpu_info, id));
         }
     }
     if (!allOK && errorMessage)
@@ -226,7 +225,7 @@ static void pickCompatibleGpus(const gmx_gpu_info_t *gpu_info,
     for (int i = 0; i < gpu_info->n_dev; i++)
     {
         GMX_ASSERT(gpu_info->gpu_dev, "Invalid gpu_info->gpu_dev");
-        if (getGpuCompatibilityStatus(gpu_info, i) == egpuCompatible)
+        if (isGpuCompatible(gpu_info, i))
         {
             gpu_opt->dev_compatible[gpu_opt->n_dev_compatible] = i;
             gpu_opt->n_dev_compatible++;