clang-tidy-11 fixes for hardware
authorejjordan <ejjordan@kth.se>
Mon, 19 Apr 2021 18:36:55 +0000 (20:36 +0200)
committerPascal Merz <pascal.merz@me.com>
Mon, 19 Apr 2021 23:08:39 +0000 (23:08 +0000)
Split off from !1433

src/gromacs/hardware/cpuinfo.cpp
src/gromacs/hardware/cpuinfo.h
src/gromacs/hardware/device_management_common.cpp
src/gromacs/hardware/hardwaretopology.cpp
src/gromacs/hardware/hardwaretopology.h
src/gromacs/hardware/identifyavx512fmaunits.cpp
src/gromacs/hardware/printhardware.cpp
src/gromacs/hardware/tests/.clang-tidy [new file with mode: 0644]
src/gromacs/hardware/tests/cpuinfo.cpp
src/gromacs/hardware/tests/hardwaretopology.cpp

index 15fa3cdbda6fda784c0c8976ceb42c047034e11b..82c50f0e19d6025efa9e23a4895cbbbc48c97cb9 100644 (file)
@@ -2,7 +2,7 @@
  * This file is part of the GROMACS molecular simulation package.
  *
  * Copyright (c) 2012-2018, The GROMACS development team.
- * Copyright (c) 2019,2020, by the GROMACS development team, led by
+ * Copyright (c) 2019,2020,2021, by the GROMACS development team, led by
  * Mark Abraham, David van der Spoel, Berk Hess, and Erik Lindahl,
  * and including many others, as listed in the AUTHORS file in the
  * top-level source directory and at http://www.gromacs.org.
@@ -810,14 +810,14 @@ CpuInfo::Vendor detectProcCpuInfoVendor(const std::map<std::string, std::string>
 
     // For each label in /proc/cpuinfo, compare the value to the name in the
     // testNames map above, and if it's a match return the vendor.
-    for (auto& l : { "vendor_id", "vendor", "manufacture", "model", "processor", "cpu" })
+    for (const auto& l : { "vendor_id", "vendor", "manufacture", "model", "processor", "cpu" })
     {
         if (cpuInfo.count(l) != 0U)
         {
             // there was a line with this left-hand side in /proc/cpuinfo
             const std::string& s1 = cpuInfo.at(l);
 
-            for (auto& t : testVendors)
+            for (const auto& t : testVendors)
             {
                 const std::string& s2 = t.first;
 
@@ -865,7 +865,7 @@ void detectProcCpuInfoIbm(const std::map<std::string, std::string>& cpuInfo,
         features->insert(CpuInfo::Feature::Ibm_Qpx);
     }
 
-    for (auto& l : { "model name", "model", "Processor", "cpu" })
+    for (const auto& l : { "model name", "model", "Processor", "cpu" })
     {
         if (cpuInfo.count(l) != 0U)
         {
index 00158fd4dc479f7094d75c2712c9e3dfd7020e04..a6b9041f36f8a328b9f561973bcc4f0c2617c8f3 100644 (file)
@@ -1,7 +1,7 @@
 /*
  * This file is part of the GROMACS molecular simulation package.
  *
- * Copyright (c) 2015,2016,2017,2018,2019,2020, by the GROMACS development team, led by
+ * Copyright (c) 2015,2016,2017,2018,2019,2020,2021, by the GROMACS development team, led by
  * Mark Abraham, David van der Spoel, Berk Hess, and Erik Lindahl,
  * and including many others, as listed in the AUTHORS file in the
  * top-level source directory and at http://www.gromacs.org.
@@ -162,7 +162,6 @@ public:
         int hwThreadRankInCore;  //!< Relative rank of logical processor in its core
     };
 
-public:
     /*! \brief Perform detection and construct a CpuInfo class from the results.
      *
      *  \note The detection should generally be performed again in different
index ad6cb0026f8ed266f001045bbac7b345649531af..8c7485d1df03179bf1480dd1aa87ce859323b68c 100644 (file)
@@ -156,7 +156,7 @@ void serializeDeviceInformations(const std::vector<std::unique_ptr<DeviceInforma
                        "DeviceInformation for OpenCL/SYCL can not be serialized");
     int numDevices = deviceInfoList.size();
     serializer->doInt(&numDevices);
-    for (auto& deviceInfo : deviceInfoList)
+    for (const auto& deviceInfo : deviceInfoList)
     {
         serializer->doOpaque(reinterpret_cast<char*>(deviceInfo.get()), sizeof(DeviceInformation));
     }
index 3659bc4e52e25f71dfbeed05e6a1c572ee9c5805..1aac4b344de4fda21205325fbee9d64a875f1063 100644 (file)
@@ -2,7 +2,7 @@
  * This file is part of the GROMACS molecular simulation package.
  *
  * Copyright (c) 2012,2013,2014,2015,2016 by the GROMACS development team.
- * Copyright (c) 2017,2018,2019,2020, by the GROMACS development team, led by
+ * Copyright (c) 2017,2018,2019,2020,2021, by the GROMACS development team, led by
  * Mark Abraham, David van der Spoel, Berk Hess, and Erik Lindahl,
  * and including many others, as listed in the AUTHORS file in the
  * top-level source directory and at http://www.gromacs.org.
@@ -106,7 +106,7 @@ void parseCpuInfo(HardwareTopology::Machine* machine, HardwareTopology::SupportL
         int nHwThreads = 0;
 
         // Copy the logical processor information from cpuinfo
-        for (auto& l : cpuInfo.logicalProcessors())
+        for (const auto& l : cpuInfo.logicalProcessors())
         {
             machine->logicalProcessors.push_back(
                     { l.socketRankInMachine, l.coreRankInSocket, l.hwThreadRankInCore, -1 });
index b52ad0a3b06887fcb4ab2ab3ce2947536c5482f4..065cb1d42bc76acc381b5d8af395bb4d61914811 100644 (file)
@@ -1,7 +1,7 @@
 /*
  * This file is part of the GROMACS molecular simulation package.
  *
- * Copyright (c) 2015,2016,2018,2019, by the GROMACS development team, led by
+ * Copyright (c) 2015,2016,2018,2019,2021, by the GROMACS development team, led by
  * Mark Abraham, David van der Spoel, Berk Hess, and Erik Lindahl,
  * and including many others, as listed in the AUTHORS file in the
  * top-level source directory and at http://www.gromacs.org.
@@ -186,7 +186,6 @@ public:
         std::vector<Device>           devices;           //!< Devices on PCI bus
     };
 
-public:
     /*! \brief Detects the hardware topology. */
     static HardwareTopology detect();
 
index 4138130eef92e6ed1c5d6d882491b02b7f9b8fab..b7ae52707c3ce0457c91c216ced8de92fbf0689c 100644 (file)
@@ -1,7 +1,7 @@
 /*
  * This file is part of the GROMACS molecular simulation package.
  *
- * Copyright (c) 2017,2018,2019,2020, by the GROMACS development team, led by
+ * Copyright (c) 2017,2018,2019,2020,2021, by the GROMACS development team, led by
  * Mark Abraham, David van der Spoel, Berk Hess, and Erik Lindahl,
  * and including many others, as listed in the AUTHORS file in the
  * top-level source directory and at http://www.gromacs.org.
@@ -292,6 +292,7 @@ bool checkDualAvx512FmaUnits()
  * We only execute the test once, and return the saved result
  * on subsequent calls.
  */
+// NOLINTNEXTLINE(cppcoreguidelines-avoid-non-const-global-variables)
 std::mutex initMutex;
 
 } // namespace
index 767b4f12b82af44c1eecc9d469c79b4ad729c829..32a5eedda6e15d545dc5ab0f8da93e34ea0dc364 100644 (file)
@@ -2,7 +2,7 @@
  * This file is part of the GROMACS molecular simulation package.
  *
  * Copyright (c) 2012,2013,2014,2015,2016, The GROMACS development team.
- * Copyright (c) 2017,2018,2019,2020, by the GROMACS development team, led by
+ * Copyright (c) 2017,2018,2019,2020,2021, by the GROMACS development team, led by
  * Mark Abraham, David van der Spoel, Berk Hess, and Erik Lindahl,
  * and including many others, as listed in the AUTHORS file in the
  * top-level source directory and at http://www.gromacs.org.
@@ -237,7 +237,7 @@ static std::string detected_hardware_string(const gmx_hw_info_t* hwinfo, bool bF
                                cpuInfo.stepping());
 
         s += gmx::formatString("    Features:");
-        for (auto& f : cpuInfo.featureSet())
+        for (const auto& f : cpuInfo.featureSet())
         {
             s += gmx::formatString(" %s", gmx::CpuInfo::featureString(f).c_str());
         }
@@ -292,13 +292,13 @@ static std::string detected_hardware_string(const gmx_hw_info_t* hwinfo, bool bF
         {
             s += gmx::formatString("    Sockets, cores, and logical processors:\n");
 
-            for (auto& socket : hwTop.machine().sockets)
+            for (const auto& socket : hwTop.machine().sockets)
             {
                 s += gmx::formatString("      Socket %2d:", socket.id);
-                for (auto& c : socket.cores)
+                for (const auto& c : socket.cores)
                 {
                     s += gmx::formatString(" [");
-                    for (auto& t : c.hwThreads)
+                    for (const auto& t : c.hwThreads)
                     {
                         s += gmx::formatString(" %3d", t.logicalProcessorId);
                     }
@@ -310,10 +310,10 @@ static std::string detected_hardware_string(const gmx_hw_info_t* hwinfo, bool bF
         if (hwTop.supportLevel() >= gmx::HardwareTopology::SupportLevel::Full)
         {
             s += gmx::formatString("    Numa nodes:\n");
-            for (auto& n : hwTop.machine().numa.nodes)
+            for (const auto& n : hwTop.machine().numa.nodes)
             {
                 s += gmx::formatString("      Node %2d (%zu bytes mem):", n.id, n.memory);
-                for (auto& l : n.logicalProcessorId)
+                for (const auto& l : n.logicalProcessorId)
                 {
                     s += gmx::formatString(" %3d", l);
                 }
@@ -337,7 +337,7 @@ static std::string detected_hardware_string(const gmx_hw_info_t* hwinfo, bool bF
 
 
             s += gmx::formatString("    Caches:\n");
-            for (auto& c : hwTop.machine().caches)
+            for (const auto& c : hwTop.machine().caches)
             {
                 s += gmx::formatString(
                         "      L%d: %zu bytes, linesize %d bytes, assoc. %d, shared %d ways\n",
@@ -351,7 +351,7 @@ static std::string detected_hardware_string(const gmx_hw_info_t* hwinfo, bool bF
         if (hwTop.supportLevel() >= gmx::HardwareTopology::SupportLevel::FullWithDevices)
         {
             s += gmx::formatString("    PCI devices:\n");
-            for (auto& d : hwTop.machine().devices)
+            for (const auto& d : hwTop.machine().devices)
             {
                 s += gmx::formatString(
                         "      %04x:%02x:%02x.%1x  Id: %04x:%04x  Class: 0x%04x  Numa: %d\n",
diff --git a/src/gromacs/hardware/tests/.clang-tidy b/src/gromacs/hardware/tests/.clang-tidy
new file mode 100644 (file)
index 0000000..0adf51e
--- /dev/null
@@ -0,0 +1,91 @@
+# List of rationales for check suppressions (where known).
+# This have to precede the list because inline comments are not
+# supported by clang-tidy.
+#
+#         -cppcoreguidelines-non-private-member-variables-in-classes,
+#         -misc-non-private-member-variables-in-classes,
+# We intend a gradual transition to conform to this guideline, but it
+# is not practical to implement yet.
+#
+#         -readability-isolate-declaration,
+# Declarations like "int a, b;" are readable. Some forms are not, and
+# those might reasonably be suggested against during code review.
+#
+#         -cppcoreguidelines-avoid-c-arrays,
+# C arrays are still necessary in many places with legacy code
+#
+#         -cppcoreguidelines-avoid-magic-numbers,
+#         -readability-magic-numbers,
+# We have many legitimate use cases for magic numbers
+#
+#         -cppcoreguidelines-macro-usage,
+# We do use too many macros, and we should fix many of them, but there
+# is no reasonable way to suppress the check e.g. in src/config.h and
+# configuring the build is a major legitimate use of macros.
+#
+#         -cppcoreguidelines-narrowing-conversions,
+#         -bugprone-narrowing-conversions
+# We have many cases where int is converted to float and we don't care
+# enough about such potential loss of precision to use explicit casts
+# in large numbers of places.
+#
+#         -google-readability-avoid-underscore-in-googletest-name
+# We need to use underscores for readability for our legacy types
+# and command-line parameter names
+#
+#         -misc-no-recursion
+# We have way too many functions and methods relying on recursion
+#
+#         -cppcoreguidelines-avoid-non-const-global-variables
+# There are quite a lot of static variables in the test code that
+# can not be replaced.
+#
+#         -modernize-avoid-bind
+# Some code needs to use std::bind and can't be modernized quickly.
+Checks:  clang-diagnostic-*,-clang-analyzer-*,-clang-analyzer-security.insecureAPI.strcpy,
+         bugprone-*,misc-*,readability-*,performance-*,mpi-*,
+         -readability-inconsistent-declaration-parameter-name,
+         -readability-function-size,-readability-else-after-return,
+         modernize-use-nullptr,modernize-use-emplace,
+         modernize-make-unique,modernize-make-shared,
+         modernize-avoid-bind,
+         modernize-use-override,
+         modernize-redundant-void-arg,modernize-use-bool-literals,
+         cppcoreguidelines-*,-cppcoreguidelines-pro-*,-cppcoreguidelines-owning-memory,
+         -cppcoreguidelines-no-malloc,-cppcoreguidelines-special-member-functions,
+         -cppcoreguidelines-avoid-goto,
+         google-*,-google-build-using-namespace,-google-explicit-constructor,
+         -google-readability-function-size,-google-readability-todo,-google-runtime-int,
+         -cppcoreguidelines-non-private-member-variables-in-classes,
+         -misc-non-private-member-variables-in-classes,
+         -readability-isolate-declaration,
+         -cppcoreguidelines-avoid-c-arrays,
+         -cppcoreguidelines-avoid-magic-numbers,
+         -readability-magic-numbers,
+         -cppcoreguidelines-macro-usage,
+         -cppcoreguidelines-narrowing-conversions,
+         -bugprone-narrowing-conversions,
+         -google-readability-avoid-underscore-in-googletest-name,
+         -cppcoreguidelines-init-variables,
+         -misc-no-recursion,
+         -cppcoreguidelines-avoid-non-const-global-variables,
+         -modernize-avoid-bind
+HeaderFilterRegex: .*
+CheckOptions:
+  - key:           cppcoreguidelines-special-member-functions.AllowSoleDefaultDtor
+    value:         1
+  - key:           modernize-make-unique.IncludeStyle
+    value:         google
+  - key:           modernize-make-shared.IncludeStyle
+    value:         google
+  - key:           readability-implicit-bool-conversion.AllowIntegerConditions
+    value:         1
+  - key:           readability-implicit-bool-conversion.AllowPointerConditions
+    value:         1
+  - key:           bugprone-dangling-handle.HandleClasses
+    value:         std::basic_string_view; nonstd::sv_lite::basic_string_view
+# Permit passing shard pointers by value for sink parameters
+  - key:           performance-unnecessary-copy-initialization.AllowedTypes
+    value:         shared_ptr
+  - key:           performance-unnecessary-value-param.AllowedTypes
+    value:         shared_ptr
index 53937307d614a75ae9d80386b795f25ef15a71b5..d0ea6a73b663d472c45d4285cabb38224788cd1d 100644 (file)
@@ -1,7 +1,7 @@
 /*
  * This file is part of the GROMACS molecular simulation package.
  *
- * Copyright (c) 2015,2016,2019, by the GROMACS development team, led by
+ * Copyright (c) 2015,2016,2019,2021, by the GROMACS development team, led by
  * Mark Abraham, David van der Spoel, Berk Hess, and Erik Lindahl,
  * and including many others, as listed in the AUTHORS file in the
  * top-level source directory and at http://www.gromacs.org.
@@ -76,7 +76,7 @@ TEST(CpuInfoTest, SupportLevel)
     if (c.supportLevel() >= gmx::CpuInfo::SupportLevel::LogicalProcessorInfo)
     {
         // Make sure assigned numbers are reasonable if we have them
-        for (auto& l : c.logicalProcessors())
+        for (const auto& l : c.logicalProcessors())
         {
             EXPECT_GE(l.socketRankInMachine, 0)
                     << "Impossible socket index for logical processor. " << commonMsg << std::endl;
index 7f7fb076b73f7ac3d6e4bd702c56ef6a7adf2570..92a18641dd6c2fddabdc76545a1f0b109527f91b 100644 (file)
@@ -1,7 +1,7 @@
 /*
  * This file is part of the GROMACS molecular simulation package.
  *
- * Copyright (c) 2015,2016,2018,2019,2020, by the GROMACS development team, led by
+ * Copyright (c) 2015,2016,2018,2019,2020,2021, by the GROMACS development team, led by
  * Mark Abraham, David van der Spoel, Berk Hess, and Erik Lindahl,
  * and including many others, as listed in the AUTHORS file in the
  * top-level source directory and at http://www.gromacs.org.
@@ -163,7 +163,7 @@ TEST(HardwareTopologyTest, NumaCacheSelfconsistency)
 
         // Check that the sum of numa domains is the total processor count
         int processorsinNumaNudes = 0;
-        for (auto& n : hwTop.machine().numa.nodes)
+        for (const auto& n : hwTop.machine().numa.nodes)
         {
             processorsinNumaNudes += n.logicalProcessorId.size();
         }
@@ -175,9 +175,9 @@ TEST(HardwareTopologyTest, NumaCacheSelfconsistency)
         {
             elem = 0;
         }
-        for (auto& n : hwTop.machine().numa.nodes)
+        for (const auto& n : hwTop.machine().numa.nodes)
         {
-            for (auto& idx : n.logicalProcessorId)
+            for (const auto& idx : n.logicalProcessorId)
             {
                 v[idx] = 1;
             }
@@ -186,7 +186,7 @@ TEST(HardwareTopologyTest, NumaCacheSelfconsistency)
         EXPECT_EQ(uniqueProcessorsinNumaNudes, hwTop.machine().logicalProcessorCount);
 
         // We must have some memory in a numa node
-        for (auto& n : hwTop.machine().numa.nodes)
+        for (const auto& n : hwTop.machine().numa.nodes)
         {
             EXPECT_GT(n.memory, 0);
         }
@@ -196,11 +196,11 @@ TEST(HardwareTopologyTest, NumaCacheSelfconsistency)
         EXPECT_GT(hwTop.machine().numa.maxRelativeLatency, 0);
         // Check number of rows matches # numa nodes
         EXPECT_EQ(hwTop.machine().numa.relativeLatency.size(), hwTop.machine().numa.nodes.size());
-        for (auto& v2 : hwTop.machine().numa.relativeLatency)
+        for (const auto& v2 : hwTop.machine().numa.relativeLatency)
         {
             // Check that size of each row matches # numa nodes
             EXPECT_EQ(v2.size(), hwTop.machine().numa.nodes.size());
-            for (auto& latency : v2)
+            for (const auto& latency : v2)
             {
                 // Latency values should be positive
                 EXPECT_GT(latency, 0);