Update cppcheck to 1.85 (ROCm#1358)

* Update cppcheck version * Fix several cppcheck issues * Formatting * Fix some knownConditionTrue issues * Formatting * Call JoinStrings * Formatting * Remove stray semicolon * remove unused variable
apwojcik · Nov 27, 2018 · 7f20999 · 7f20999
1 parent 17a492c
commit 7f20999
Show file tree

Hide file tree

Showing 16 changed files with 52 additions and 71 deletions.
diff --git a/cmake/CppCheck.cmake b/cmake/CppCheck.cmake
@@ -96,7 +96,7 @@ macro(enable_cppcheck)
             ${CPPCHECK_FORCE}
             --cppcheck-build-dir=${CPPCHECK_BUILD_DIR}
             --platform=native
-            \"--template={file}:{line}: {severity}: {message} [{id}]\"
+            --template=gcc
             --error-exitcode=1
             -j ${CPPCHECK_JOBS}
             ${CPPCHECK_DEFINES}

diff --git a/dev-requirements.txt b/dev-requirements.txt
@@ -1,4 +1,4 @@
 pfultz2/rocm-recipes
 ROCm-Developer-Tools/HIP@3a41f286203968421c557338d6fb39c36f3c717c
 -f requirements.txt
-danmar/cppcheck@fe1b1d537ecaa0e573000f7d073a0e661e3256f2
+danmar/cppcheck@1.85
diff --git a/src/fusion.cpp b/src/fusion.cpp
@@ -1057,10 +1057,12 @@ std::vector<Exec_arg_t> FusionPlanDescriptor::CalcArgOrder(Handle& handle)
             {
                 auto keys = ptr_map.at(idx);
                 std::sort(keys.begin(), keys.end());
-                for(auto& key : keys)
-                {
-                    arg_keys.emplace_back(key, Pointer, sizeof(ConstData_t));
-                }
+                std::transform(keys.begin(),
+                               keys.end(),
+                               std::back_inserter(arg_keys),
+                               [&](auto&& key) -> Exec_arg_t {
+                                   return {key, Pointer, sizeof(ConstData_t)};
+                               });
             }
         }
         if(kernel_source_type == AsmText)

diff --git a/src/include/miopen/hipoc_kernel.hpp b/src/include/miopen/hipoc_kernel.hpp
@@ -111,11 +111,7 @@ struct KernelArgsPack<>
 template <class... Ts>
 struct KernelArgs
 {
-    KernelArgs(Ts... xs) : pack(xs...)
-    {
-        for(auto& x : hidden)
-            x = 0;
-    }
+    KernelArgs(Ts... xs) : pack(xs...) { std::fill(std::begin(hidden), std::end(hidden), 0); }
     KernelArgsPack<Ts...> pack;
     uint64_t hidden[6] = {};
 };

diff --git a/src/include/miopen/logger.hpp b/src/include/miopen/logger.hpp
@@ -26,6 +26,7 @@
 #ifndef GUARD_MIOPEN_LOGGER_HPP
 #define GUARD_MIOPEN_LOGGER_HPP
 
+#include <algorithm>
 #include <array>
 #include <iostream>
 #include <miopen/each_args.hpp>
@@ -166,15 +167,11 @@ std::array<T, sizeof...(Ts) + 1> make_array(T x, Ts... xs)
 template <class T, class Range>
 std::ostream& LogEnum(std::ostream& os, T x, Range&& values)
 {
-    for(auto&& p : values)
-    {
-        if(p.second == x)
-        {
-            os << p.first;
-            return os;
-        }
-    }
-    os << "Unknown: " << x;
+    auto it = std::find_if(values.begin(), values.end(), [&](auto&& p) { return p.second == x; });
+    if(it == values.end())
+        os << "Unknown: " << x;
+    else
+        os << it->first;
     return os;
 }
 

diff --git a/src/include/miopen/solver.hpp b/src/include/miopen/solver.hpp
@@ -272,12 +272,9 @@ auto SearchForSolution(const Context& search_params, Db db) ->
 template <class Solution>
 static inline bool IsPureOpenCLSolution(const Solution& s)
 {
-    for(auto& k : s.construction_params)
-    {
-        if(!miopen::EndsWith(k.kernel_file, ".cl"))
-            return false;
-    }
-    return true;
+    return std::all_of(s.construction_params.begin(),
+                       s.construction_params.end(),
+                       [](const auto& k) { return miopen::EndsWith(k.kernel_file, ".cl"); });
 }
 
 // Search for all applicable solutions among many solvers

diff --git a/src/kernel_warnings.cpp b/src/kernel_warnings.cpp
@@ -26,6 +26,8 @@
 #include <iterator>
 #include <miopen/config.h>
 #include <miopen/kernel_warnings.hpp>
+#include <miopen/stringutils.hpp>
+#include <numeric>
 #include <sstream>
 
 namespace miopen {
@@ -51,16 +53,13 @@ std::vector<std::string> KernelWarnings()
 
 std::string MakeKernelWarningsString()
 {
-    std::string result;
 #if MIOPEN_BACKEND_OPENCL
     std::string prefix = " -Wf,";
 
 #else
     std::string prefix = " ";
 #endif
-    for(auto&& x : KernelWarnings())
-        result += prefix + x;
-    return result;
+    return prefix + JoinStrings(KernelWarnings(), prefix);
 }
 
 const std::string& KernelWarningsString()

diff --git a/src/md_graph.cpp b/src/md_graph.cpp
@@ -191,14 +191,13 @@ bool FusionMDGraph::SetConvAlgo(miopenConvFwdAlgorithm_t algo)
 
     for(auto& kinder : cur_vertex)
     {
-        MDGraph_vertex_ptr& cur_vertex_ptr = kinder.first;
-        auto& cur_map                      = kinder.second;
+        auto& cur_map = kinder.second;
         if(cur_map.find("algo") != cur_map.end())
         {
             miopenConvFwdAlgorithm_t a = boost::any_cast<miopenConvFwdAlgorithm_t>(cur_map["algo"]);
             if(a == algo)
             {
-                new_list.emplace_back(cur_vertex_ptr, cur_map);
+                new_list.emplace_back(kinder.first, cur_map);
             }
         }
         else
@@ -1088,19 +1087,23 @@ bool FusionMDGraph::CmpOpKey(const FusionMDGraph_Edge_Map& edge_val,
             auto op_val = op->MDGraphKey();
             if(op_val.count(kv.first) == 1)
             {
-                for(auto& edg_ops : kv.second)
+                auto edg_op_it =
+                    std::find_if(kv.second.begin(), kv.second.end(), [&](auto&& edg_ops) {
+                        return !FusionMDGraph::ExecEdgeOp(edg_ops, op_val.at(kv.first).at(0));
+                    });
+                if(edg_op_it == kv.second.end())
                 {
-                    if(!FusionMDGraph::ExecEdgeOp(edg_ops, op_val.at(kv.first).at(0)))
-                    {
-                        MIOPEN_LOG_I2("Edge Op :" << edg_ops << " Op Val: "
-                                                  << op_val.at(kv.first).at(0)
-                                                  << " Edge Op for key: "
-                                                  << kv.first
-                                                  << " Failed");
-                        return false;
-                    }
+                    MIOPEN_LOG_I2("Edge Op for key: " << kv.first << " Successfull");
+                }
+                else
+                {
+                    MIOPEN_LOG_I2("Edge Op :" << *edg_op_it << " Op Val: "
+                                              << op_val.at(kv.first).at(0)
+                                              << " Edge Op for key: "
+                                              << kv.first
+                                              << " Failed");
+                    return false;
                 }
-                MIOPEN_LOG_I2("Edge Op for key: " << kv.first << " Successfull");
             }
             else
             {

diff --git a/src/ocl/convolutionocl.cpp b/src/ocl/convolutionocl.cpp
@@ -1016,6 +1016,7 @@ void ConvolutionDescriptor::FindConvFwdAlgorithm(Handle& handle,
     ProblemDescription problem(xDesc, wDesc, yDesc, *this, 1);
 
     const auto find_db_path = GetFindDbPath() + "/" + handle.GetDbPathFilename() + ".cd.fdb.txt";
+    (void)find_db_path;
     auto record =
         boost::optional<DbRecord>{boost::none}; // Db{find_db_path, false}.FindRecord(problem);
     auto loaded = record.is_initialized();

diff --git a/src/solver/conv_asm_3x3u.cpp b/src/solver/conv_asm_3x3u.cpp
@@ -104,9 +104,8 @@ bool PerformanceConfigConvAsm3x3U::IsValid(const ConvolutionContext& config) con
     assert(active_lanes != 0);
     if(active_lanes == 0)
         return false;
-    const bool uneven_line_read_mode  = (img_x_blocks % active_lanes != 0);
-    const bool uneven_line_write_mode = (img_width % active_lanes != 0);
-    if(uneven_line_read_mode || uneven_line_write_mode)
+    const bool uneven_line_read_mode = (img_x_blocks % active_lanes != 0);
+    if(uneven_line_read_mode)
         ++n;
 
     const int block_size_x        = 1;

diff --git a/src/solver/conv_ocl_dir2D11x11.cpp b/src/solver/conv_ocl_dir2D11x11.cpp
@@ -124,6 +124,7 @@ ConvSolution ConvOclDirectFwd11x11::GetSolution(const ConvolutionContext& params
 
     // param
     // 6 get us the min
+    // cppcheck-suppress knownConditionTrueFalse
     static const int backwards_min_output = (data_multiplier1 > 1 || data_multiplier0 > 1) ? 1 : 4;
     result.n_out_pix_tiles                = (is_forward)
                                  ? std::min(6, (params.n_outputs + n_out_stacks - 1) / n_out_stacks)

diff --git a/test/args.hpp b/test/args.hpp
@@ -29,6 +29,7 @@
 #include <functional>
 
 #include <miopen/each_args.hpp>
+#include <numeric>
 #include <sstream>
 #include <stdexcept>
 #include <string>
@@ -274,12 +275,11 @@ struct read_value
               ARGS_REQUIRES(is_container<Container>{} and not is_output_streamable<Container>{})>
     std::string operator()(Container& xs) const
     {
-        std::string result;
-        for(auto&& x : xs)
-        {
-            result += (*this)(x) + " ";
-        }
-        return result;
+        if(xs.begin() == xs.end())
+            return "";
+        return std::accumulate(xs.begin() + 1, xs.end(), (*this)(*xs.begin()), [&](auto x, auto y) {
+            return x + " " + (*this)(y);
+        });
     }
 
     template <class T, ARGS_REQUIRES(is_output_streamable<T>{})>

diff --git a/test/gru.cpp b/test/gru.cpp
@@ -2853,9 +2853,7 @@ struct gru_driver : test_driver
             std::cout << "batch seq[" << i << "]: " << batchSeq.at(i) << std::endl;
         }
 #endif
-        int batch_n = 0;
-        for(auto& n : batchSeq)
-            batch_n += n;
+        int batch_n = std::accumulate(batchSeq.begin(), batchSeq.end(), 0);
 
         miopenRNNDescriptor_t rnnDesc;
         miopenCreateRNNDescriptor(&rnnDesc);

diff --git a/test/lstm.cpp b/test/lstm.cpp
@@ -2669,9 +2669,7 @@ struct lstm_driver : test_driver
 
         auto&& handle = get_handle();
 
-        int batch_n = 0;
-        for(auto& n : batchSeq)
-            batch_n += n;
+        int batch_n = std::accumulate(batchSeq.begin(), batchSeq.end(), 0);
 
         miopenRNNDescriptor_t rnnDesc;
         miopenCreateRNNDescriptor(&rnnDesc);

diff --git a/test/rnn_util.hpp b/test/rnn_util.hpp
@@ -108,15 +108,7 @@ inline std::vector<std::vector<int>> generate_batchSeq(const int batchSize, cons
     return {batchSeq};
 }
 
-inline int sumvc(const std::vector<int>& x)
-{
-    int sum = 0;
-    for(int i : x)
-    {
-        sum += i;
-    }
-    return sum;
-}
+inline int sumvc(const std::vector<int>& x) { return std::accumulate(x.begin(), x.end(), 0); }
 
 inline float activfunc(float x, int actvf)
 {

diff --git a/test/rnn_vanilla.cpp b/test/rnn_vanilla.cpp
@@ -2269,9 +2269,7 @@ struct rnn_vanilla_driver : test_driver
 
         auto&& handle = get_handle();
 
-        int batch_n = 0;
-        for(auto& n : batchSeq)
-            batch_n += n;
+        int batch_n = std::accumulate(batchSeq.begin(), batchSeq.end(), 0);
 
         miopenRNNDescriptor_t rnnDesc;
         miopenCreateRNNDescriptor(&rnnDesc);