Skip to content

Commit 08d8755

Browse files
[CPU] Introduce clang-tidy (openvinotoolkit#28040)
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
1 parent 4b1f824 commit 08d8755

File tree

407 files changed

+1899
-1636
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

407 files changed

+1899
-1636
lines changed

.github/dockerfiles/docker_tag

+1-1
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
pr-28380
1+
pr-28040

.github/dockerfiles/ov_build/ubuntu_22_04_x64_cc/Dockerfile

+7-3
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,11 @@ RUN apt-get update && \
3636
# For Java API
3737
default-jdk \
3838
# Compiler \
39-
clang \
39+
clang-15 \
40+
# Static analyzer
41+
clang-tidy-15 \
42+
# clang-tidy uses clang-format as a dependency
43+
clang-format-15 \
4044
&& \
4145
rm -rf /var/lib/apt/lists/*
4246

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

4953
# Set clang as a default compiler
50-
RUN update-alternatives --install /usr/bin/cc cc /usr/bin/clang 100 && \
51-
update-alternatives --install /usr/bin/c++ c++ /usr/bin/clang++ 100
54+
RUN update-alternatives --install /usr/bin/cc cc /usr/bin/clang-15 100 && \
55+
update-alternatives --install /usr/bin/c++ c++ /usr/bin/clang++-15 100
5256

5357
# Install sscache
5458
ARG SCCACHE_VERSION="v0.7.5"

.github/workflows/linux_conditional_compilation.yml

+4-1
Original file line numberDiff line numberDiff line change
@@ -151,14 +151,17 @@ jobs:
151151
# Build
152152
#
153153

154-
- name: CMake configure - CC COLLECT
154+
- name: CMake configure - CC COLLECT with clang-tidy
155+
# clang-tidy static analysis check is enabled as part of collection
156+
# to avoid an additional separate build execution
155157
run: |
156158
cmake \
157159
-G "${{ env.CMAKE_GENERATOR }}" \
158160
-DCMAKE_CXX_STANDARD=20 \
159161
-DBUILD_SHARED_LIBS=OFF \
160162
-DENABLE_TESTS=ON \
161163
-DENABLE_CPPLINT=OFF \
164+
-DENABLE_CLANG_TIDY=ON \
162165
-DENABLE_NCC_STYLE=OFF \
163166
-DCMAKE_COMPILE_WARNING_AS_ERROR=ON \
164167
-DENABLE_PROFILING_ITT=ON \

cmake/developer_package/OpenVINODeveloperScriptsConfig.cmake

+1
Original file line numberDiff line numberDiff line change
@@ -305,6 +305,7 @@ include(python_requirements)
305305

306306
include(cpplint/cpplint)
307307
include(clang_format/clang_format)
308+
include(clang_tidy/clang_tidy)
308309
include(ncc_naming_style/ncc_naming_style)
309310

310311
# Restore state
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
# Copyright (C) 2018-2024 Intel Corporation
2+
# SPDX-License-Identifier: Apache-2.0
3+
#
4+
5+
if(ENABLE_CLANG_TIDY)
6+
set(CLANG_TIDY_REQUIRED_VERSION 15 CACHE STRING "clang-tidy version to use")
7+
set(CLANG_TIDY_FILENAME clang-tidy-${CLANG_TIDY_REQUIRED_VERSION} clang-tidy)
8+
find_host_program(CLANG_TIDY NAMES ${CLANG_TIDY_FILENAME} PATHS ENV PATH)
9+
if(CLANG_TIDY)
10+
execute_process(COMMAND ${CLANG_TIDY} ${CMAKE_CURRENT_SOURCE_DIR} ARGS --version OUTPUT_VARIABLE CLANG_VERSION)
11+
if(NOT CLANG_VERSION)
12+
message(WARNING "Supported clang-tidy version is ${CLANG_TIDY_REQUIRED_VERSION}!")
13+
set(ENABLE_CLANG_TIDY OFF)
14+
else()
15+
string(REGEX REPLACE "[^0-9]+([0-9]+)\\..*" "\\1" CLANG_TIDY_MAJOR_VERSION ${CLANG_VERSION})
16+
if(NOT CLANG_TIDY_MAJOR_VERSION EQUAL CLANG_TIDY_REQUIRED_VERSION)
17+
message(WARNING "Supported clang-tidy version is ${CLANG_TIDY_REQUIRED_VERSION}! Provided version ${CLANG_TIDY_MAJOR_VERSION}")
18+
set(ENABLE_CLANG_TIDY OFF)
19+
endif()
20+
endif()
21+
else()
22+
message(WARNING "Supported clang-tidy-${CLANG_TIDY_REQUIRED_VERSION} is not found!")
23+
set(ENABLE_CLANG_TIDY OFF)
24+
endif()
25+
endif()

cmake/developer_package/features.cmake

+2
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,8 @@ ov_dependent_option (ENABLE_CPPLINT_REPORT "Build cpplint report instead of fail
7878

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

81+
ov_option (ENABLE_CLANG_TIDY "Enable clang-tidy checks during the build" ${STYLE_CHECKS_DEFAULT})
82+
8183
ov_option (ENABLE_NCC_STYLE "Enable ncc style check" ${STYLE_CHECKS_DEFAULT})
8284

8385
ov_option (ENABLE_UNSAFE_LOCATIONS "skip check for MD5 for dependency" OFF)

cmake/developer_package/plugins/plugins.cmake

+8-1
Original file line numberDiff line numberDiff line change
@@ -34,10 +34,11 @@ endif()
3434
# [SKIP_INSTALL]
3535
# [SKIP_REGISTRATION] Skip creation of <device>.xml
3636
# [ADD_CLANG_FORMAT]
37+
# [ADD_CLANG_TIDY]
3738
# )
3839
#
3940
function(ov_add_plugin)
40-
set(options SKIP_INSTALL PSEUDO_DEVICE ADD_CLANG_FORMAT AS_EXTENSION SKIP_REGISTRATION)
41+
set(options SKIP_INSTALL PSEUDO_DEVICE ADD_CLANG_FORMAT ADD_CLANG_TIDY AS_EXTENSION SKIP_REGISTRATION)
4142
set(oneValueArgs NAME DEVICE_NAME VERSION_DEFINES_FOR PSEUDO_PLUGIN_FOR)
4243
set(multiValueArgs DEFAULT_CONFIG SOURCES OBJECT_LIBRARIES CPPLINT_FILTERS)
4344
cmake_parse_arguments(OV_PLUGIN "${options}" "${oneValueArgs}" "${multiValueArgs}" ${ARGN})
@@ -105,6 +106,12 @@ function(ov_add_plugin)
105106
string(CONCAT custom_filter "${custom_filter}" "," "${filter}")
106107
endforeach()
107108

109+
if (OV_PLUGIN_ADD_CLANG_TIDY)
110+
if (ENABLE_CLANG_TIDY)
111+
set_target_properties(${OV_PLUGIN_NAME} PROPERTIES CXX_CLANG_TIDY clang-tidy-${CLANG_TIDY_REQUIRED_VERSION})
112+
endif()
113+
endif()
114+
108115
if (OV_PLUGIN_ADD_CLANG_FORMAT)
109116
ov_add_clang_format_target(${OV_PLUGIN_NAME}_clang FOR_SOURCES ${OV_PLUGIN_SOURCES})
110117
else()

src/plugins/intel_cpu/CMakeLists.txt

+2-1
Original file line numberDiff line numberDiff line change
@@ -239,7 +239,8 @@ ov_add_plugin(NAME ${TARGET_NAME}
239239
AS_EXTENSION
240240
VERSION_DEFINES_FOR src/plugin.cpp
241241
SOURCES ${SOURCES} ${HEADERS}
242-
ADD_CLANG_FORMAT)
242+
ADD_CLANG_FORMAT
243+
ADD_CLANG_TIDY)
243244

244245
# give a different file name depending on target platform architecture
245246
if(ARM OR AARCH64)

src/plugins/intel_cpu/src/.clang-tidy

+83
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,83 @@
1+
---
2+
3+
### NOTE:
4+
# The 'Checks: >' is a multiline string here. Comment must not be moved into the string.
5+
#
6+
### Scopes to be enabled:
7+
#
8+
# cppcoreguidelines-*,
9+
# google-*,
10+
# readability-*,
11+
# modernize-*,
12+
# bugprone-*,
13+
# misc-*,
14+
#
15+
### Checks that are turned off for a reason:
16+
#
17+
# -cppcoreguidelines-pro-bounds-pointer-arithmetic
18+
# -google-readability-todo. No big reason to enforce
19+
# -modernize-use-trailing-return-type. Just stylistic preference
20+
# -readability-identifier-length. A lot of code use short names for readability, i.e. 'B' for batch
21+
# -readability-uppercase-literal-suffix.
22+
#
23+
### Checks that are turned off but better be enabled later:
24+
# -bugprone-narrowing-conversions
25+
# -bugprone-easily-swappable-parameters
26+
# -bugprone-fold-init-type
27+
# -bugprone-implicit-widening-of-multiplication-result
28+
# -cppcoreguidelines-narrowing-conversions
29+
# -google-readability-braces-around-statements
30+
# -readability-implicit-bool-conversion,
31+
# -readability-magic-numbers, cppcoreguidelines-avoid-magic-numbers
32+
# -readability-function-cognitive-complexity. Reasonable way to enforce splitting complex code into simple functions
33+
# -modernize-concat-nested-namespaces. More compact way when C++17 is available
34+
35+
Checks: >
36+
-*,
37+
performance-*,
38+
modernize-pass-by-value,
39+
cppcoreguidelines-prefer-member-initializer,
40+
-bugprone-easily-swappable-parameters,
41+
-bugprone-fold-init-type,
42+
-bugprone-implicit-widening-of-multiplication-result,
43+
-bugprone-narrowing-conversions,
44+
-cppcoreguidelines-narrowing-conversions,
45+
-cppcoreguidelines-pro-bounds-pointer-arithmetic,
46+
-google-build-using-namespace,
47+
-google-readability-todo,
48+
-readability-braces-around-statements,
49+
-google-readability-braces-around-statements,
50+
-modernize-use-trailing-return-type,
51+
-readability-identifier-length,
52+
-readability-implicit-bool-conversion,
53+
-readability-magic-numbers,
54+
-cppcoreguidelines-avoid-magic-numbers,
55+
-readability-uppercase-literal-suffix,
56+
-readability-function-cognitive-complexity,
57+
-modernize-concat-nested-namespaces,
58+
# Treat warnings as errors
59+
WarningsAsErrors: '*'
60+
# Use clang-format for applied fixes
61+
FormatStyle: file
62+
HeaderFilterRegex: ''
63+
CheckOptions:
64+
- key: cppcoreguidelines-avoid-do-while.IgnoreMacros
65+
value: true
66+
# matches with corresponding cpplink check
67+
- key: google-readability-namespace-comments.ShortNamespaceLines
68+
value: "10"
69+
# matches with corresponding cpplink check
70+
- key: google-readability-namespace-comments.SpacesBeforeComments
71+
value: "2"
72+
- key: modernize-loop-convert.MinConfidence
73+
value: reasonable
74+
- key: modernize-pass-by-value.IncludeStyle
75+
value: google
76+
### To be considered to enable:
77+
# # Unifies the usage of the statements
78+
# - key: readability-braces-around-statements.ShortStatementLines
79+
# value: "1"
80+
# Reasonable way to enforce splitting complex code into simple functions
81+
# - key: google-readability-function-size.StatementThreshold
82+
# value: "800"
83+
---

src/plugins/intel_cpu/src/compiled_model.cpp

+19-17
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
#include <utility>
99

1010
#include "async_infer_request.h"
11+
#include "config.h"
1112
#include "cpu/x64/cpu_isa_traits.hpp"
1213
#include "infer_request.h"
1314
#include "itt.h"
@@ -44,34 +45,34 @@ struct ImmediateSerialExecutor : public ov::threading::ITaskExecutor {
4445

4546
CompiledModel::CompiledModel(const std::shared_ptr<ov::Model>& model,
4647
const std::shared_ptr<const ov::IPlugin>& plugin,
47-
const Config& cfg,
48+
Config cfg,
4849
const bool loaded_from_cache,
49-
const std::shared_ptr<SubMemoryManager> sub_memory_manager)
50+
std::shared_ptr<SubMemoryManager> sub_memory_manager)
5051
: ov::ICompiledModel::ICompiledModel(model, plugin),
5152
m_model(model),
5253
m_plugin(plugin),
53-
m_cfg{cfg},
54+
m_cfg{std::move(cfg)},
5455
m_name{model->get_name()},
5556
m_loaded_from_cache(loaded_from_cache),
56-
m_sub_memory_manager(sub_memory_manager) {
57+
m_sub_memory_manager(std::move(sub_memory_manager)) {
5758
m_mutex = std::make_shared<std::mutex>();
5859
const auto& core = m_plugin->get_core();
5960
if (!core)
6061
OPENVINO_THROW("Unable to get API version. Core is unavailable");
6162

62-
IStreamsExecutor::Config executor_confg;
63-
if (cfg.exclusiveAsyncRequests) {
63+
IStreamsExecutor::Config executor_config;
64+
if (m_cfg.exclusiveAsyncRequests) {
6465
// special case when all InferRequests are muxed into a single queue
6566
m_task_executor = m_plugin->get_executor_manager()->get_executor("CPU");
6667
} else {
67-
executor_confg = m_cfg.numSubStreams > 0 ? IStreamsExecutor::Config{"CPUMainStreamExecutor",
68-
1,
69-
1,
70-
ov::hint::SchedulingCoreType::ANY_CORE,
71-
false,
72-
true}
73-
: m_cfg.streamExecutorConfig;
74-
m_task_executor = m_plugin->get_executor_manager()->get_idle_cpu_streams_executor(executor_confg);
68+
executor_config = m_cfg.numSubStreams > 0 ? IStreamsExecutor::Config{"CPUMainStreamExecutor",
69+
1,
70+
1,
71+
ov::hint::SchedulingCoreType::ANY_CORE,
72+
false,
73+
true}
74+
: m_cfg.streamExecutorConfig;
75+
m_task_executor = m_plugin->get_executor_manager()->get_idle_cpu_streams_executor(executor_config);
7576
}
7677
if (0 != m_cfg.streamExecutorConfig.get_streams()) {
7778
m_callback_executor = m_plugin->get_executor_manager()->get_idle_cpu_streams_executor(
@@ -85,11 +86,11 @@ CompiledModel::CompiledModel(const std::shared_ptr<ov::Model>& model,
8586
if (m_callback_executor)
8687
set_callback_executor(m_callback_executor);
8788

88-
int streams = std::max(1, executor_confg.get_streams());
89+
int streams = std::max(1, executor_config.get_streams());
8990
std::vector<Task> tasks;
9091
tasks.resize(streams);
9192
m_graphs.resize(streams);
92-
if (executor_confg.get_streams() != 0) {
93+
if (executor_config.get_streams() != 0) {
9394
auto all_graphs_ready = [&] {
9495
return std::all_of(m_graphs.begin(), m_graphs.end(), [&](Graph& graph) {
9596
return graph.IsReady();
@@ -196,7 +197,8 @@ std::shared_ptr<ov::IAsyncInferRequest> CompiledModel::create_infer_request() co
196197
get_callback_executor());
197198
if (m_has_sub_compiled_models) {
198199
std::vector<std::shared_ptr<IAsyncInferRequest>> requests;
199-
for (auto model : m_sub_compiled_models) {
200+
requests.reserve(m_sub_compiled_models.size());
201+
for (const auto& model : m_sub_compiled_models) {
200202
requests.push_back(model->create_infer_request());
201203
}
202204
async_infer_request->setSubInferRequest(requests);

src/plugins/intel_cpu/src/compiled_model.h

+2-2
Original file line numberDiff line numberDiff line change
@@ -34,9 +34,9 @@ class CompiledModel : public ov::ICompiledModel {
3434

3535
CompiledModel(const std::shared_ptr<ov::Model>& model,
3636
const std::shared_ptr<const ov::IPlugin>& plugin,
37-
const Config& cfg,
37+
Config cfg,
3838
const bool loaded_from_cache,
39-
const std::shared_ptr<SubMemoryManager> sub_memory_manager = nullptr);
39+
std::shared_ptr<SubMemoryManager> sub_memory_manager = nullptr);
4040

4141
~CompiledModel() {
4242
if (m_has_sub_compiled_models) {

src/plugins/intel_cpu/src/cpu_map_scheduling.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ std::vector<std::vector<int>> apply_scheduling_core_type(ov::hint::SchedulingCor
4848

4949
std::vector<std::vector<int>> apply_hyper_threading(bool& input_ht_hint,
5050
const bool input_ht_changed,
51-
const std::string input_pm_hint,
51+
const std::string& input_pm_hint,
5252
const std::vector<std::vector<int>>& proc_type_table) {
5353
std::vector<std::vector<int>> result_table = proc_type_table;
5454

src/plugins/intel_cpu/src/cpu_map_scheduling.hpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ std::vector<std::vector<int>> apply_scheduling_core_type(ov::hint::SchedulingCor
3737
*/
3838
std::vector<std::vector<int>> apply_hyper_threading(bool& input_ht_hint,
3939
const bool input_ht_changed,
40-
const std::string input_pm_hint,
40+
const std::string& input_pm_hint,
4141
const std::vector<std::vector<int>>& proc_type_table);
4242

4343
/**

0 commit comments

Comments
 (0)