diff --git a/.clang-tidy b/.clang-tidy index 14ccb31af..89c56a96a 100644 --- a/.clang-tidy +++ b/.clang-tidy @@ -1,49 +1,23 @@ --- -# Create new issues to allow -# -readability-math-missing-parentheses -# eg) '*' has higher precedence than '+'; add parentheses to explicitly specify the order of operations -# -readability-avoid-const-params-in-decls : -# eg) std::vector UniqueNames(const std::function& variables, const std::size_t i)> f) const; -# -readability-non-const-parameter; -# eg) double* d_lower_matrix (micm/src/solver/linear_solver.cu) -# -readability-convert-member-functions-to-static -# -readability-identifier-length -# -readability-braces-around-statements -# -readability-make-member-function-const - -# Need discussion -# -readability-implicit-bool-conversion: We could allow this while accepeting the two conditions specified in `Checks` -# eg) assert((!generated_) && "JIT Function already generated") (jit/jit_function.hpp) -# -readability-isolate-declaration -# eg) double d_ymax, d_scale -> multiple declarations in a single statement reduces readability (micm/src/solver/rosenbrock.cu) -# -readability-named-parameter: false positive cases - -# Have to fix later -# -readability-avoid-unconditional-preprocessor-if: -# eg) #if 0 (micm/jit/jit_compiler.hpp) - -# Disabled -# -readability-simplify-boolean-expr: Could hurt readability -# eg) return static_cast(elem == end) -# -readability-magic-numbers,-warnings-as-errors: Doesn't allow the following example -# eg) parameters.c_[8] = -0.3399990352819905E+02; (micm/solver/rosenbrock_solver_parameters.hpp) +# Here is an explanation for why some of the checks are disabled: +# +# -readability-identifier-length: +# : We want to enable this but this requires large cleanup efforts. +# +# -readability-convert-member-functions-to-static: +# : This wants to put 'static' in the inlined function defined outside the class definition +# but 'static' must be specified inside the class definition. Checks: > -*, readability-*, - -readability-named-parameter, - -readability-identifier-length, - -readability-convert-member-functions-to-static, - -readability-braces-around-statements, - -readability-math-missing-parentheses, -readability-avoid-const-params-in-decls, - -readability-avoid-unconditional-preprocessor-if, - -readability-make-member-function-const, - -readability-implicit-bool-conversion, - -readability-simplify-boolean-expr, -readability-magic-numbers,-warnings-as-errors, - -readability-non-const-parameter, -readability-isolate-declaration, + -readability-avoid-unconditional-preprocessor-if, + -readability-identifier-length, + -readability-convert-member-functions-to-static, + -readability-named-parameter, bugprone-*, -bugprone-signal-handler, cert-*, @@ -74,4 +48,4 @@ CheckOptions: - key: readability-implicit-bool-conversion.AllowIntegerConditions value: 1 - key: readability-implicit-bool-conversion.AllowPointerConditions - value: 1 + value: 1 \ No newline at end of file diff --git a/include/micm/jit/jit_compiler.hpp b/include/micm/jit/jit_compiler.hpp index f890f455d..66e0c7cba 100644 --- a/include/micm/jit/jit_compiler.hpp +++ b/include/micm/jit/jit_compiler.hpp @@ -205,11 +205,6 @@ namespace micm for (auto &function : module) { pass_manager->run(function); -#if 0 - std::cout << "Generated function definition:" << std::endl; - function.print(llvm::errs()); - std::cout << std::endl; -#endif } }); diff --git a/include/micm/jit/jit_function.hpp b/include/micm/jit/jit_function.hpp index c01c1b158..6f7ad7a44 100644 --- a/include/micm/jit/jit_function.hpp +++ b/include/micm/jit/jit_function.hpp @@ -133,7 +133,7 @@ namespace micm void EndLoop(JitLoop& loop); private: - llvm::AllocaInst* CreateEntryBlockAlloca(llvm::Type* type, const std::string& var_name); + llvm::AllocaInst* CreateEntryBlockAlloca(llvm::Type* type, const std::string& var_name) const; }; class JitFunctionBuilder @@ -187,8 +187,9 @@ namespace micm arg.arg_ = arg_iter++; arg.arg_->setName(arg.name_); } - for (unsigned int i = 0; i < arguments_.size(); ++i) + for (unsigned int i = 0; i < arguments_.size(); ++i) { function_->addParamAttr(i, llvm::Attribute::NoAlias); + } // function body @@ -207,7 +208,7 @@ namespace micm std::pair JitFunction::Generate() { - assert((!generated_) && "JIT Function already generated"); + assert((!generated_) && static_cast("JIT Function already generated")); std::pair ret_val; verifyFunction(*function_); ret_val.first = compiler_->GetMainJITDylib().createResourceTracker(); @@ -288,7 +289,7 @@ namespace micm loop.index_->addIncoming(nextIter, loop.block_); } - inline llvm::AllocaInst* JitFunction::CreateEntryBlockAlloca(llvm::Type* type, const std::string& var_name) + inline llvm::AllocaInst* JitFunction::CreateEntryBlockAlloca(llvm::Type* type, const std::string& var_name) const { llvm::IRBuilder<> TmpB(&function_->getEntryBlock(), function_->getEntryBlock().begin()); return TmpB.CreateAlloca(type, 0, var_name.c_str()); diff --git a/include/micm/process/branched_rate_constant.hpp b/include/micm/process/branched_rate_constant.hpp index 3cfcdc313..adc854506 100644 --- a/include/micm/process/branched_rate_constant.hpp +++ b/include/micm/process/branched_rate_constant.hpp @@ -107,8 +107,9 @@ namespace micm { double pre = parameters_.X_ * std::exp(-parameters_.Y_ / temperature); double Atmn = A(temperature, air_number_density); - if (parameters_.branch_ == BranchedRateConstantParameters::Branch::Alkoxy) + if (parameters_.branch_ == BranchedRateConstantParameters::Branch::Alkoxy) { return pre * (z_ / (z_ + Atmn)); + } return pre * (Atmn / (Atmn + z_)); } diff --git a/include/micm/process/jit_process_set.hpp b/include/micm/process/jit_process_set.hpp index 9f8eacf7f..57482881a 100644 --- a/include/micm/process/jit_process_set.hpp +++ b/include/micm/process/jit_process_set.hpp @@ -259,8 +259,9 @@ namespace micm // d_rate_d_ind[i_cell] *= reactant_concentration for each reactant except ind for (std::size_t i_react = 0; i_react < number_of_reactants_[i_rxn]; ++i_react) { - if (i_react == i_ind) + if (i_react == i_ind) { continue; + } loop = func.StartLoop("d_rate_d_ind calc", 0, L); llvm::Value *react_id = llvm::ConstantInt::get(*(func.context_), llvm::APInt(64, react_ids[i_react] * L)); ptr_index[0] = func.builder_->CreateNSWAdd(loop.index_, react_id); diff --git a/include/micm/process/process_set.hpp b/include/micm/process/process_set.hpp index 5b1305a1d..2c65a9632 100644 --- a/include/micm/process/process_set.hpp +++ b/include/micm/process/process_set.hpp @@ -239,14 +239,17 @@ namespace micm { double rate = cell_rate_constants[i_rxn]; - for (std::size_t i_react = 0; i_react < number_of_reactants_[i_rxn]; ++i_react) + for (std::size_t i_react = 0; i_react < number_of_reactants_[i_rxn]; ++i_react) { rate *= cell_state[react_id[i_react]]; + } - for (std::size_t i_react = 0; i_react < number_of_reactants_[i_rxn]; ++i_react) + for (std::size_t i_react = 0; i_react < number_of_reactants_[i_rxn]; ++i_react) { cell_forcing[react_id[i_react]] -= rate; + } - for (std::size_t i_prod = 0; i_prod < number_of_products_[i_rxn]; ++i_prod) + for (std::size_t i_prod = 0; i_prod < number_of_products_[i_rxn]; ++i_prod) { cell_forcing[prod_id[i_prod]] += yield[i_prod] * rate; + } react_id += number_of_reactants_[i_rxn]; prod_id += number_of_products_[i_rxn]; @@ -281,7 +284,7 @@ namespace micm std::vector rate(L, 0); for (std::size_t i_rxn = 0; i_rxn < number_of_reactants_.size(); ++i_rxn) { - auto v_rate_subrange_begin = v_rate_constants_begin + offset_rc + i_rxn * L; + auto v_rate_subrange_begin = v_rate_constants_begin + offset_rc + (i_rxn * L); rate.assign(v_rate_subrange_begin, v_rate_subrange_begin + L); for (std::size_t i_react = 0; i_react < number_of_reactants_[i_rxn]; ++i_react) for (std::size_t i_cell = 0; i_cell < L; ++i_cell) @@ -332,14 +335,17 @@ namespace micm for (std::size_t i_react = 0; i_react < number_of_reactants_[i_rxn]; ++i_react) { - if (i_react == i_ind) + if (i_react == i_ind) { continue; + } d_rate_d_ind *= cell_state[react_id[i_react]]; } - for (std::size_t i_dep = 0; i_dep < number_of_reactants_[i_rxn]; ++i_dep) + for (std::size_t i_dep = 0; i_dep < number_of_reactants_[i_rxn]; ++i_dep) { cell_jacobian[*(flat_id++)] += d_rate_d_ind; - for (std::size_t i_dep = 0; i_dep < number_of_products_[i_rxn]; ++i_dep) + } + for (std::size_t i_dep = 0; i_dep < number_of_products_[i_rxn]; ++i_dep) { cell_jacobian[*(flat_id++)] -= yield[i_dep] * d_rate_d_ind; + } } react_id += number_of_reactants_[i_rxn]; yield += number_of_products_[i_rxn]; @@ -381,13 +387,14 @@ namespace micm { for (std::size_t i_ind = 0; i_ind < number_of_reactants_[i_rxn]; ++i_ind) { - auto v_rate_subrange_begin = v_rate_constants_begin + offset_rc + i_rxn * L; + auto v_rate_subrange_begin = v_rate_constants_begin + offset_rc + (i_rxn * L); d_rate_d_ind.assign(v_rate_subrange_begin, v_rate_subrange_begin + L); for (std::size_t i_react = 0; i_react < number_of_reactants_[i_rxn]; ++i_react) { - if (i_react == i_ind) + if (i_react == i_ind) { continue; - std::size_t idx_state_variables = offset_state + react_id[i_react] * L; + } + std::size_t idx_state_variables = offset_state + (react_id[i_react] * L); for (std::size_t i_cell = 0; i_cell < L; ++i_cell) d_rate_d_ind[i_cell] *= v_state_variables[idx_state_variables + i_cell]; } diff --git a/include/micm/profiler/instrumentation.hpp b/include/micm/profiler/instrumentation.hpp index 30c8c2e56..1c2e75c3e 100644 --- a/include/micm/profiler/instrumentation.hpp +++ b/include/micm/profiler/instrumentation.hpp @@ -148,8 +148,9 @@ namespace micm ~InstrumentationTimer() { - if (!stopped_) + if (!stopped_) { Stop(); + } } void Stop() @@ -188,10 +189,12 @@ namespace micm while (srcIndex < N) { size_t matchIndex = 0; - while (matchIndex < K - 1 && srcIndex + matchIndex < N - 1 && expr[srcIndex + matchIndex] == remove[matchIndex]) + while (matchIndex < K - 1 && srcIndex + matchIndex < N - 1 && expr[srcIndex + matchIndex] == remove[matchIndex]) { matchIndex++; - if (matchIndex == K - 1) + } + if (matchIndex == K - 1) { srcIndex += matchIndex; + } result.data_[dstIndex++] = expr[srcIndex] == '"' ? '\'' : expr[srcIndex]; srcIndex++; } diff --git a/include/micm/solver/jit_lu_decomposition.hpp b/include/micm/solver/jit_lu_decomposition.hpp index 027148210..9bd40dc27 100644 --- a/include/micm/solver/jit_lu_decomposition.hpp +++ b/include/micm/solver/jit_lu_decomposition.hpp @@ -29,8 +29,8 @@ namespace micm JitLuDecomposition(const JitLuDecomposition &) = delete; JitLuDecomposition &operator=(const JitLuDecomposition &) = delete; - JitLuDecomposition(JitLuDecomposition &&); - JitLuDecomposition &operator=(JitLuDecomposition &&); + JitLuDecomposition(JitLuDecomposition &&other); + JitLuDecomposition &operator=(JitLuDecomposition &&other); /// @brief Create a JITed LU decomposer for a given sparse matrix structure /// @param compiler JIT compiler diff --git a/include/micm/util/cuda_dense_matrix.hpp b/include/micm/util/cuda_dense_matrix.hpp index 17600d7e8..be8ab0162 100644 --- a/include/micm/util/cuda_dense_matrix.hpp +++ b/include/micm/util/cuda_dense_matrix.hpp @@ -180,8 +180,9 @@ namespace micm ~CudaDenseMatrix() requires(std::is_same_v) { CHECK_CUDA_ERROR(micm::cuda::FreeVector(this->param_), "cudaFree"); - if (this->handle_ != NULL) + if (this->handle_ != NULL) { cublasDestroy(this->handle_); + } this->param_.d_data_ = nullptr; this->handle_ = NULL; } diff --git a/include/micm/util/jacobian.hpp b/include/micm/util/jacobian.hpp index 55a394fee..a8f3202c6 100644 --- a/include/micm/util/jacobian.hpp +++ b/include/micm/util/jacobian.hpp @@ -25,9 +25,9 @@ namespace micm for (auto& elem : nonzero_jacobian_elements) builder = builder.WithElement(elem.first, elem.second); // Always include diagonal elements - for (std::size_t i = 0; i < state_size; ++i) + for (std::size_t i = 0; i < state_size; ++i) { builder = builder.WithElement(i, i); - + } return SparseMatrixPolicy(builder); } } // namespace micm diff --git a/include/micm/util/sparse_matrix.hpp b/include/micm/util/sparse_matrix.hpp index 62462cebf..5ed2a6285 100644 --- a/include/micm/util/sparse_matrix.hpp +++ b/include/micm/util/sparse_matrix.hpp @@ -210,9 +210,7 @@ namespace micm auto begin = std::next(row_ids_.begin(), row_start_[row]); auto end = std::next(row_ids_.begin(), row_start_[row + 1]); auto elem = std::find(begin, end, column); - if (elem == end) - return true; - return false; + return (elem == end); } std::size_t NumberOfBlocks() const diff --git a/include/micm/util/vector_matrix.hpp b/include/micm/util/vector_matrix.hpp index 099fc7f8c..dad64be79 100644 --- a/include/micm/util/vector_matrix.hpp +++ b/include/micm/util/vector_matrix.hpp @@ -251,12 +251,15 @@ namespace micm auto y_iter = data_.begin(); auto x_iter = x.AsVector().begin(); const std::size_t n = std::floor(x_dim_ / L) * L * y_dim_; - for (std::size_t i = 0; i < n; ++i) + for (std::size_t i = 0; i < n; ++i) { *(y_iter++) += alpha * (*(x_iter++)); + } const std::size_t l = x_dim_ % L; - for (std::size_t i = 0; i < y_dim_; ++i) - for (std::size_t j = 0; j < l; ++j) + for (std::size_t i = 0; i < y_dim_; ++i) { + for (std::size_t j = 0; j < l; ++j) { y_iter[(i * L) + j] += alpha * x_iter[(i * L) + j]; + } + } } void ForEach(const std::function f, const VectorMatrix &a) @@ -264,12 +267,15 @@ namespace micm auto this_iter = data_.begin(); auto a_iter = a.AsVector().begin(); const std::size_t n = std::floor(x_dim_ / L) * L * y_dim_; - for (std::size_t i = 0; i < n; ++i) + for (std::size_t i = 0; i < n; ++i) { f(*(this_iter++), *(a_iter++)); + } const std::size_t l = x_dim_ % L; - for (std::size_t y = 0; y < y_dim_; ++y) - for (std::size_t x = 0; x < l; ++x) + for (std::size_t y = 0; y < y_dim_; ++y) { + for (std::size_t x = 0; x < l; ++x) { f(this_iter[(y * L) + x], a_iter[(y * L) + x]); + } + } } void ForEach(const std::function f, const VectorMatrix &a, const VectorMatrix &b) @@ -280,14 +286,17 @@ namespace micm auto a_iter = a.AsVector().begin(); auto b_iter = b.AsVector().begin(); const std::size_t n = std::floor(x_dim_ / L) * L * y_dim_; - for (std::size_t i = 0; i < n; ++i) + for (std::size_t i = 0; i < n; ++i) { f(*(this_iter++), *(a_iter++), *(b_iter++)); + } const std::size_t l = x_dim_ % L; if (l > 0) { - for (std::size_t y = 0; y < y_dim_; ++y) - for (std::size_t x = 0; x < l; ++x) + for (std::size_t y = 0; y < y_dim_; ++y) { + for (std::size_t x = 0; x < l; ++x) { f(this_iter[(y * L) + x], a_iter[(y * L) + x], b_iter[(y * L) + x]); + } + } } } diff --git a/src/process/process_set.cu b/src/process/process_set.cu index 11643fd50..dfd8fd4ee 100644 --- a/src/process/process_set.cu +++ b/src/process/process_set.cu @@ -39,12 +39,13 @@ namespace micm yield_offset = 0; for (std::size_t i_rxn = 0; i_rxn < number_of_reactions; ++i_rxn) { - double rate = d_rate_constants[i_rxn * number_of_grid_cells + tid]; - for (std::size_t i_react = 0; i_react < d_number_of_reactants[i_rxn]; ++i_react) - rate *= d_state_variables[d_reactant_ids[react_id_offset + i_react] * number_of_grid_cells + tid]; + double rate = d_rate_constants[(i_rxn * number_of_grid_cells) + tid]; + for (std::size_t i_react = 0; i_react < d_number_of_reactants[i_rxn]; ++i_react) { + rate *= d_state_variables[(d_reactant_ids[react_id_offset + i_react] * number_of_grid_cells) + tid]; + } for (std::size_t i_react = 0; i_react < d_number_of_reactants[i_rxn]; ++i_react) { - d_forcing[d_reactant_ids[react_id_offset + i_react] * number_of_grid_cells + tid] -= rate; + d_forcing[(d_reactant_ids[react_id_offset + i_react] * number_of_grid_cells) + tid] -= rate; } for (std::size_t i_prod = 0; i_prod < d_number_of_products[i_rxn]; ++i_prod) { @@ -92,12 +93,12 @@ namespace micm // loop over reactants in a reaction for (size_t i_ind = 0; i_ind < d_number_of_reactants[i_rxn]; ++i_ind) { - double d_rate_d_ind = d_rate_constants[i_rxn * number_of_grid_cells + tid]; + double d_rate_d_ind = d_rate_constants[(i_rxn * number_of_grid_cells) + tid]; for (size_t i_react = 0; i_react < d_number_of_reactants[i_rxn]; ++i_react) { if (i_react != i_ind) { - d_rate_d_ind *= d_state_variables[d_reactant_ids[react_ids_offset + i_react] * number_of_grid_cells + tid]; + d_rate_d_ind *= d_state_variables[(d_reactant_ids[react_ids_offset + i_react] * number_of_grid_cells) + tid]; } } for (size_t i_dep = 0; i_dep < d_number_of_reactants[i_rxn]; ++i_dep) @@ -189,8 +190,9 @@ namespace micm cudaFree(devstruct.number_of_products_); cudaFree(devstruct.product_ids_); cudaFree(devstruct.yields_); - if (devstruct.jacobian_flat_ids_ != nullptr) + if (devstruct.jacobian_flat_ids_ != nullptr) { cudaFree(devstruct.jacobian_flat_ids_); + } } void SubtractJacobianTermsKernelDriver( diff --git a/src/solver/linear_solver.cu b/src/solver/linear_solver.cu index f997ee2fa..daa4998dc 100644 --- a/src/solver/linear_solver.cu +++ b/src/solver/linear_solver.cu @@ -48,15 +48,15 @@ namespace micm for (size_t j = 0; j < nLij_Lii_size; ++j) { auto& nLij_Lii_element = d_nLij_Lii[j]; - d_y[y_column_index * number_of_grid_cells + tid] = d_b[b_column_index++ * number_of_grid_cells + tid]; + d_y[(y_column_index * number_of_grid_cells) + tid] = d_b[(b_column_index++ * number_of_grid_cells) + tid]; for (size_t i = 0; i < nLij_Lii_element.first; ++i) { size_t lower_matrix_index = d_Lij_yj[Lij_yj_index].first + tid; size_t y_index = d_Lij_yj[Lij_yj_index].second * number_of_grid_cells + tid; - d_y[y_column_index * number_of_grid_cells + tid] -= d_L[lower_matrix_index] * d_y[y_index]; + d_y[(y_column_index * number_of_grid_cells) + tid] -= d_L[lower_matrix_index] * d_y[y_index]; ++Lij_yj_index; } - d_y[y_column_index++ * number_of_grid_cells + tid] /= d_L[nLij_Lii_element.second + tid]; + d_y[(y_column_index++ * number_of_grid_cells) + tid] /= d_L[nLij_Lii_element.second + tid]; } for (size_t k = 0; k < nUij_Uii_size; ++k) @@ -67,10 +67,10 @@ namespace micm { size_t upper_matrix_index = d_Uij_xj[Uij_xj_index].first + tid; size_t x_index = d_Uij_xj[Uij_xj_index].second * number_of_grid_cells + tid; - d_x[x_column_backward_index * number_of_grid_cells + tid] -= d_U[upper_matrix_index] * d_x[x_index]; + d_x[(x_column_backward_index * number_of_grid_cells) + tid] -= d_U[upper_matrix_index] * d_x[x_index]; ++Uij_xj_index; } - d_x[x_column_backward_index * number_of_grid_cells + tid] /= d_U[nUij_Uii_element.second + tid]; + d_x[(x_column_backward_index * number_of_grid_cells) + tid] /= d_U[nUij_Uii_element.second + tid]; if (x_column_backward_index != 0) { diff --git a/src/solver/rosenbrock.cu b/src/solver/rosenbrock.cu index 499322ec0..98f69bfb0 100644 --- a/src/solver/rosenbrock.cu +++ b/src/solver/rosenbrock.cu @@ -84,8 +84,9 @@ namespace micm // No need to synchronize threads in the same warp __device__ void WarpReduce(volatile double* sdata, size_t tid) { - if (BLOCK_SIZE >= 64) + if (BLOCK_SIZE >= 64) { sdata[tid] += sdata[tid + 32]; + } sdata[tid] += sdata[tid + 16]; sdata[tid] += sdata[tid + 8]; sdata[tid] += sdata[tid + 4]; @@ -148,11 +149,13 @@ namespace micm // Load two elements by one thread and do first add of reduction // Access the d_errors array directly if it is not the first call sdata[l_tid] = 0.0; - if (g_tid < n) + if (g_tid < n) { sdata[l_tid] += d_errors_input[g_tid]; + } g_tid += BLOCK_SIZE; - if (g_tid < n) + if (g_tid < n) { sdata[l_tid] += d_errors_input[g_tid]; + } __syncthreads(); } @@ -189,8 +192,9 @@ namespace micm } __syncthreads(); } - if (l_tid < 32) + if (l_tid < 32) { WarpReduce(sdata, l_tid); + } // Let the thread 0 of this threadblock write its result to output array, inexed by this threadblock if (l_tid == 0)