Skip to content

Commit

Permalink
[CPU] Introduce clang-tidy (openvinotoolkit#28040)
Browse files Browse the repository at this point in the history
For now .clang-tidy file is placed into intel_cpu/src plugin folded,
since
if we put it into root src folder, this would show clang-tidy warnings
in IDEs for every source file of the openvino project.
From cmake configuration point of view clang-tidy check is implemented
as a generic one.

Regarding clang-tidy checks, for now only `performance-*` checks are
enabled and fixed.
Additionally, the following two checks were enabled, since they are
conflicting with the `performance-unnecessary-value-param` check

```
  modernize-pass-by-value,
  cppcoreguidelines-prefer-member-initializer,
```

The idea is to enable the check scopes one by one, since fixing them is
quite time consuming.

As for pre-commit check, the clang-tidy is enabled in scope of
`linux_conditional_compilation` check to avoid an additional build
execution.

Only x64 arch is covered
aarch64 will be enabled next
  • Loading branch information
EgorDuplensky authored Jan 17, 2025
1 parent 4b1f824 commit 08d8755
Show file tree
Hide file tree
Showing 407 changed files with 1,899 additions and 1,636 deletions.
2 changes: 1 addition & 1 deletion .github/dockerfiles/docker_tag
Original file line number Diff line number Diff line change
@@ -1 +1 @@
pr-28380
pr-28040
10 changes: 7 additions & 3 deletions .github/dockerfiles/ov_build/ubuntu_22_04_x64_cc/Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,11 @@ RUN apt-get update && \
# For Java API
default-jdk \
# Compiler \
clang \
clang-15 \
# Static analyzer
clang-tidy-15 \
# clang-tidy uses clang-format as a dependency
clang-format-15 \
&& \
rm -rf /var/lib/apt/lists/*

Expand All @@ -47,8 +51,8 @@ RUN chmod +x /install_build_dependencies.sh && \
rm -rf /var/lib/apt/lists/*

# Set clang as a default compiler
RUN update-alternatives --install /usr/bin/cc cc /usr/bin/clang 100 && \
update-alternatives --install /usr/bin/c++ c++ /usr/bin/clang++ 100
RUN update-alternatives --install /usr/bin/cc cc /usr/bin/clang-15 100 && \
update-alternatives --install /usr/bin/c++ c++ /usr/bin/clang++-15 100

# Install sscache
ARG SCCACHE_VERSION="v0.7.5"
Expand Down
5 changes: 4 additions & 1 deletion .github/workflows/linux_conditional_compilation.yml
Original file line number Diff line number Diff line change
Expand Up @@ -151,14 +151,17 @@ jobs:
# Build
#

- name: CMake configure - CC COLLECT
- name: CMake configure - CC COLLECT with clang-tidy
# clang-tidy static analysis check is enabled as part of collection
# to avoid an additional separate build execution
run: |
cmake \
-G "${{ env.CMAKE_GENERATOR }}" \
-DCMAKE_CXX_STANDARD=20 \
-DBUILD_SHARED_LIBS=OFF \
-DENABLE_TESTS=ON \
-DENABLE_CPPLINT=OFF \
-DENABLE_CLANG_TIDY=ON \
-DENABLE_NCC_STYLE=OFF \
-DCMAKE_COMPILE_WARNING_AS_ERROR=ON \
-DENABLE_PROFILING_ITT=ON \
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -305,6 +305,7 @@ include(python_requirements)

include(cpplint/cpplint)
include(clang_format/clang_format)
include(clang_tidy/clang_tidy)
include(ncc_naming_style/ncc_naming_style)

# Restore state
Expand Down
25 changes: 25 additions & 0 deletions cmake/developer_package/clang_tidy/clang_tidy.cmake
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
# Copyright (C) 2018-2024 Intel Corporation
# SPDX-License-Identifier: Apache-2.0
#

if(ENABLE_CLANG_TIDY)
set(CLANG_TIDY_REQUIRED_VERSION 15 CACHE STRING "clang-tidy version to use")
set(CLANG_TIDY_FILENAME clang-tidy-${CLANG_TIDY_REQUIRED_VERSION} clang-tidy)
find_host_program(CLANG_TIDY NAMES ${CLANG_TIDY_FILENAME} PATHS ENV PATH)
if(CLANG_TIDY)
execute_process(COMMAND ${CLANG_TIDY} ${CMAKE_CURRENT_SOURCE_DIR} ARGS --version OUTPUT_VARIABLE CLANG_VERSION)
if(NOT CLANG_VERSION)
message(WARNING "Supported clang-tidy version is ${CLANG_TIDY_REQUIRED_VERSION}!")
set(ENABLE_CLANG_TIDY OFF)
else()
string(REGEX REPLACE "[^0-9]+([0-9]+)\\..*" "\\1" CLANG_TIDY_MAJOR_VERSION ${CLANG_VERSION})
if(NOT CLANG_TIDY_MAJOR_VERSION EQUAL CLANG_TIDY_REQUIRED_VERSION)
message(WARNING "Supported clang-tidy version is ${CLANG_TIDY_REQUIRED_VERSION}! Provided version ${CLANG_TIDY_MAJOR_VERSION}")
set(ENABLE_CLANG_TIDY OFF)
endif()
endif()
else()
message(WARNING "Supported clang-tidy-${CLANG_TIDY_REQUIRED_VERSION} is not found!")
set(ENABLE_CLANG_TIDY OFF)
endif()
endif()
2 changes: 2 additions & 0 deletions cmake/developer_package/features.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,8 @@ ov_dependent_option (ENABLE_CPPLINT_REPORT "Build cpplint report instead of fail

ov_option (ENABLE_CLANG_FORMAT "Enable clang-format checks during the build" ${STYLE_CHECKS_DEFAULT})

ov_option (ENABLE_CLANG_TIDY "Enable clang-tidy checks during the build" ${STYLE_CHECKS_DEFAULT})

ov_option (ENABLE_NCC_STYLE "Enable ncc style check" ${STYLE_CHECKS_DEFAULT})

ov_option (ENABLE_UNSAFE_LOCATIONS "skip check for MD5 for dependency" OFF)
Expand Down
9 changes: 8 additions & 1 deletion cmake/developer_package/plugins/plugins.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -34,10 +34,11 @@ endif()
# [SKIP_INSTALL]
# [SKIP_REGISTRATION] Skip creation of <device>.xml
# [ADD_CLANG_FORMAT]
# [ADD_CLANG_TIDY]
# )
#
function(ov_add_plugin)
set(options SKIP_INSTALL PSEUDO_DEVICE ADD_CLANG_FORMAT AS_EXTENSION SKIP_REGISTRATION)
set(options SKIP_INSTALL PSEUDO_DEVICE ADD_CLANG_FORMAT ADD_CLANG_TIDY AS_EXTENSION SKIP_REGISTRATION)
set(oneValueArgs NAME DEVICE_NAME VERSION_DEFINES_FOR PSEUDO_PLUGIN_FOR)
set(multiValueArgs DEFAULT_CONFIG SOURCES OBJECT_LIBRARIES CPPLINT_FILTERS)
cmake_parse_arguments(OV_PLUGIN "${options}" "${oneValueArgs}" "${multiValueArgs}" ${ARGN})
Expand Down Expand Up @@ -105,6 +106,12 @@ function(ov_add_plugin)
string(CONCAT custom_filter "${custom_filter}" "," "${filter}")
endforeach()

if (OV_PLUGIN_ADD_CLANG_TIDY)
if (ENABLE_CLANG_TIDY)
set_target_properties(${OV_PLUGIN_NAME} PROPERTIES CXX_CLANG_TIDY clang-tidy-${CLANG_TIDY_REQUIRED_VERSION})
endif()
endif()

if (OV_PLUGIN_ADD_CLANG_FORMAT)
ov_add_clang_format_target(${OV_PLUGIN_NAME}_clang FOR_SOURCES ${OV_PLUGIN_SOURCES})
else()
Expand Down
3 changes: 2 additions & 1 deletion src/plugins/intel_cpu/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -239,7 +239,8 @@ ov_add_plugin(NAME ${TARGET_NAME}
AS_EXTENSION
VERSION_DEFINES_FOR src/plugin.cpp
SOURCES ${SOURCES} ${HEADERS}
ADD_CLANG_FORMAT)
ADD_CLANG_FORMAT
ADD_CLANG_TIDY)

# give a different file name depending on target platform architecture
if(ARM OR AARCH64)
Expand Down
83 changes: 83 additions & 0 deletions src/plugins/intel_cpu/src/.clang-tidy
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
---

### NOTE:
# The 'Checks: >' is a multiline string here. Comment must not be moved into the string.
#
### Scopes to be enabled:
#
# cppcoreguidelines-*,
# google-*,
# readability-*,
# modernize-*,
# bugprone-*,
# misc-*,
#
### Checks that are turned off for a reason:
#
# -cppcoreguidelines-pro-bounds-pointer-arithmetic
# -google-readability-todo. No big reason to enforce
# -modernize-use-trailing-return-type. Just stylistic preference
# -readability-identifier-length. A lot of code use short names for readability, i.e. 'B' for batch
# -readability-uppercase-literal-suffix.
#
### Checks that are turned off but better be enabled later:
# -bugprone-narrowing-conversions
# -bugprone-easily-swappable-parameters
# -bugprone-fold-init-type
# -bugprone-implicit-widening-of-multiplication-result
# -cppcoreguidelines-narrowing-conversions
# -google-readability-braces-around-statements
# -readability-implicit-bool-conversion,
# -readability-magic-numbers, cppcoreguidelines-avoid-magic-numbers
# -readability-function-cognitive-complexity. Reasonable way to enforce splitting complex code into simple functions
# -modernize-concat-nested-namespaces. More compact way when C++17 is available

Checks: >
-*,
performance-*,
modernize-pass-by-value,
cppcoreguidelines-prefer-member-initializer,
-bugprone-easily-swappable-parameters,
-bugprone-fold-init-type,
-bugprone-implicit-widening-of-multiplication-result,
-bugprone-narrowing-conversions,
-cppcoreguidelines-narrowing-conversions,
-cppcoreguidelines-pro-bounds-pointer-arithmetic,
-google-build-using-namespace,
-google-readability-todo,
-readability-braces-around-statements,
-google-readability-braces-around-statements,
-modernize-use-trailing-return-type,
-readability-identifier-length,
-readability-implicit-bool-conversion,
-readability-magic-numbers,
-cppcoreguidelines-avoid-magic-numbers,
-readability-uppercase-literal-suffix,
-readability-function-cognitive-complexity,
-modernize-concat-nested-namespaces,
# Treat warnings as errors
WarningsAsErrors: '*'
# Use clang-format for applied fixes
FormatStyle: file
HeaderFilterRegex: ''
CheckOptions:
- key: cppcoreguidelines-avoid-do-while.IgnoreMacros
value: true
# matches with corresponding cpplink check
- key: google-readability-namespace-comments.ShortNamespaceLines
value: "10"
# matches with corresponding cpplink check
- key: google-readability-namespace-comments.SpacesBeforeComments
value: "2"
- key: modernize-loop-convert.MinConfidence
value: reasonable
- key: modernize-pass-by-value.IncludeStyle
value: google
### To be considered to enable:
# # Unifies the usage of the statements
# - key: readability-braces-around-statements.ShortStatementLines
# value: "1"
# Reasonable way to enforce splitting complex code into simple functions
# - key: google-readability-function-size.StatementThreshold
# value: "800"
---
36 changes: 19 additions & 17 deletions src/plugins/intel_cpu/src/compiled_model.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
#include <utility>

#include "async_infer_request.h"
#include "config.h"
#include "cpu/x64/cpu_isa_traits.hpp"
#include "infer_request.h"
#include "itt.h"
Expand Down Expand Up @@ -44,34 +45,34 @@ struct ImmediateSerialExecutor : public ov::threading::ITaskExecutor {

CompiledModel::CompiledModel(const std::shared_ptr<ov::Model>& model,
const std::shared_ptr<const ov::IPlugin>& plugin,
const Config& cfg,
Config cfg,
const bool loaded_from_cache,
const std::shared_ptr<SubMemoryManager> sub_memory_manager)
std::shared_ptr<SubMemoryManager> sub_memory_manager)
: ov::ICompiledModel::ICompiledModel(model, plugin),
m_model(model),
m_plugin(plugin),
m_cfg{cfg},
m_cfg{std::move(cfg)},
m_name{model->get_name()},
m_loaded_from_cache(loaded_from_cache),
m_sub_memory_manager(sub_memory_manager) {
m_sub_memory_manager(std::move(sub_memory_manager)) {
m_mutex = std::make_shared<std::mutex>();
const auto& core = m_plugin->get_core();
if (!core)
OPENVINO_THROW("Unable to get API version. Core is unavailable");

IStreamsExecutor::Config executor_confg;
if (cfg.exclusiveAsyncRequests) {
IStreamsExecutor::Config executor_config;
if (m_cfg.exclusiveAsyncRequests) {
// special case when all InferRequests are muxed into a single queue
m_task_executor = m_plugin->get_executor_manager()->get_executor("CPU");
} else {
executor_confg = m_cfg.numSubStreams > 0 ? IStreamsExecutor::Config{"CPUMainStreamExecutor",
1,
1,
ov::hint::SchedulingCoreType::ANY_CORE,
false,
true}
: m_cfg.streamExecutorConfig;
m_task_executor = m_plugin->get_executor_manager()->get_idle_cpu_streams_executor(executor_confg);
executor_config = m_cfg.numSubStreams > 0 ? IStreamsExecutor::Config{"CPUMainStreamExecutor",
1,
1,
ov::hint::SchedulingCoreType::ANY_CORE,
false,
true}
: m_cfg.streamExecutorConfig;
m_task_executor = m_plugin->get_executor_manager()->get_idle_cpu_streams_executor(executor_config);
}
if (0 != m_cfg.streamExecutorConfig.get_streams()) {
m_callback_executor = m_plugin->get_executor_manager()->get_idle_cpu_streams_executor(
Expand All @@ -85,11 +86,11 @@ CompiledModel::CompiledModel(const std::shared_ptr<ov::Model>& model,
if (m_callback_executor)
set_callback_executor(m_callback_executor);

int streams = std::max(1, executor_confg.get_streams());
int streams = std::max(1, executor_config.get_streams());
std::vector<Task> tasks;
tasks.resize(streams);
m_graphs.resize(streams);
if (executor_confg.get_streams() != 0) {
if (executor_config.get_streams() != 0) {
auto all_graphs_ready = [&] {
return std::all_of(m_graphs.begin(), m_graphs.end(), [&](Graph& graph) {
return graph.IsReady();
Expand Down Expand Up @@ -196,7 +197,8 @@ std::shared_ptr<ov::IAsyncInferRequest> CompiledModel::create_infer_request() co
get_callback_executor());
if (m_has_sub_compiled_models) {
std::vector<std::shared_ptr<IAsyncInferRequest>> requests;
for (auto model : m_sub_compiled_models) {
requests.reserve(m_sub_compiled_models.size());
for (const auto& model : m_sub_compiled_models) {
requests.push_back(model->create_infer_request());
}
async_infer_request->setSubInferRequest(requests);
Expand Down
4 changes: 2 additions & 2 deletions src/plugins/intel_cpu/src/compiled_model.h
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,9 @@ class CompiledModel : public ov::ICompiledModel {

CompiledModel(const std::shared_ptr<ov::Model>& model,
const std::shared_ptr<const ov::IPlugin>& plugin,
const Config& cfg,
Config cfg,
const bool loaded_from_cache,
const std::shared_ptr<SubMemoryManager> sub_memory_manager = nullptr);
std::shared_ptr<SubMemoryManager> sub_memory_manager = nullptr);

~CompiledModel() {
if (m_has_sub_compiled_models) {
Expand Down
2 changes: 1 addition & 1 deletion src/plugins/intel_cpu/src/cpu_map_scheduling.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ std::vector<std::vector<int>> apply_scheduling_core_type(ov::hint::SchedulingCor

std::vector<std::vector<int>> apply_hyper_threading(bool& input_ht_hint,
const bool input_ht_changed,
const std::string input_pm_hint,
const std::string& input_pm_hint,
const std::vector<std::vector<int>>& proc_type_table) {
std::vector<std::vector<int>> result_table = proc_type_table;

Expand Down
2 changes: 1 addition & 1 deletion src/plugins/intel_cpu/src/cpu_map_scheduling.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ std::vector<std::vector<int>> apply_scheduling_core_type(ov::hint::SchedulingCor
*/
std::vector<std::vector<int>> apply_hyper_threading(bool& input_ht_hint,
const bool input_ht_changed,
const std::string input_pm_hint,
const std::string& input_pm_hint,
const std::vector<std::vector<int>>& proc_type_table);

/**
Expand Down
Loading

0 comments on commit 08d8755

Please sign in to comment.