Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[CPU] Enable clang-tidy-18 checks #28725

Merged
merged 3 commits into from
Feb 4, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/dockerfiles/docker_tag
Original file line number Diff line number Diff line change
@@ -1 +1 @@
pr-28040
pr-28725
6 changes: 4 additions & 2 deletions .github/dockerfiles/ov_build/ubuntu_22_04_x64_cc/Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,10 @@ ENV DEBIAN_FRONTEND="noninteractive" \
TZ="Europe/London"

RUN apt-get update && \
apt-get install software-properties-common && \
apt-get install software-properties-common wget && \
add-apt-repository --yes --no-update ppa:git-core/ppa && \
add-apt-repository --yes --no-update "deb http://apt.llvm.org/jammy/ llvm-toolchain-jammy-18 main" && \
wget -O - https://apt.llvm.org/llvm-snapshot.gpg.key | tee /etc/apt/trusted.gpg.d/llvm.asc && \
add-apt-repository --yes --no-update ppa:deadsnakes/ppa && \
apt-get update && \
apt-get install \
Expand All @@ -38,7 +40,7 @@ RUN apt-get update && \
# Compiler \
clang-15 \
# Static analyzer
clang-tidy-15 \
clang-tidy-18 \
# clang-tidy uses clang-format as a dependency
clang-format-15 \
&& \
Expand Down
2 changes: 1 addition & 1 deletion cmake/developer_package/clang_tidy/clang_tidy.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
#

if(ENABLE_CLANG_TIDY)
set(CLANG_TIDY_REQUIRED_VERSION 15 CACHE STRING "clang-tidy version to use")
set(CLANG_TIDY_REQUIRED_VERSION 18 CACHE STRING "clang-tidy version to use")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ilya-lavrenov We want to bump up the version to 18. This is a default clang version on Ubuntu24. The reason - 18 version contains a feature misc-include-cleaner, which will allow us to fix most of the include mess and implicit include dependencies we have.
After we fix the includes, we will be able to implement an independent CI workflow for clang-tidy, which will only check the changed files independently and will not require to compile all the source files with clang-tidy enabled. Without fixing the includes those file-by-file checks may fail because of missing includes.
This will allow us to implement such workflow for all the architectures with very small execution time, basically it scales much better than building the whole project with clang-tidy.

The downside is that for clang-format we will have version 15 required, but for clang-tidy - version 18. And, for example, to install clang-tidy-18 on ubuntu22 it is required to add a ppa to apt sources. From my perspective these cons are not that important, since we already support Ubuntu24 where clang-tidy-18 is the default one.

What do you think?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see a problem if we will use different versions of clang-family tools on different systems.
Once most of developers migrate to U24 as development machine, we can switch to clang-format-18

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)
Expand Down
3 changes: 2 additions & 1 deletion cmake/developer_package/plugins/plugins.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,8 @@ function(ov_add_plugin)

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})
set_target_properties(${OV_PLUGIN_NAME} PROPERTIES
CXX_CLANG_TIDY "clang-tidy-${CLANG_TIDY_REQUIRED_VERSION};--extra-arg=-Wno-unused-command-line-argument")
endif()
endif()

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ void RegPrinter::print_reg_prc(const char* name, const char* ori_name, T* ptr) {
ss << static_cast<uint64_t>(*ptr);
}
}
ss << std::endl;
ss << '\n';
std::cout << ss.str();
}

Expand All @@ -60,7 +60,7 @@ void RegPrinter::print_vmm_prc(const char* name, const char* ori_name, PRC_T* pt
for (size_t i = 1; i < vlen / sizeof(float); i++) {
ss << ", " << ptr[i];
}
ss << "}" << std::endl;
ss << "}" << '\n';
std::cout << ss.str();
}
template void RegPrinter::print_vmm_prc<float, 16>(const char* name, const char* ori_name, float* ptr);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ void jit_snippets_call_args::loop_args_t::init_pointers_and_copy_data(const int6
std::memcpy(m_finalization_offsets, finalization_offsets, chunk_size);
}

void swap(jit_snippets_call_args::loop_args_t& first, jit_snippets_call_args::loop_args_t& second) {
void swap(jit_snippets_call_args::loop_args_t& first, jit_snippets_call_args::loop_args_t& second) noexcept {
std::swap(first.m_work_amount, second.m_work_amount);
std::swap(first.m_num_data_ptrs, second.m_num_data_ptrs);
std::swap(first.m_ptr_increments, second.m_ptr_increments);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ struct jit_snippets_call_args::loop_args_t {
~loop_args_t();

loop_args_t& operator=(loop_args_t other);
friend void swap(loop_args_t& first, loop_args_t& second);
friend void swap(loop_args_t& first, loop_args_t& second) noexcept;

void init_pointers_and_copy_data(const int64_t num_elements,
const int64_t* ptr_increments,
Expand Down
18 changes: 9 additions & 9 deletions src/plugins/intel_cpu/src/graph_dumper.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -267,7 +267,7 @@ void serializeToCout(const Graph& graph) {
<< "/l=" << outConfs.front().getMemDesc()->serializeFormat();
}
}
std::cout << " ]" << std::endl;
std::cout << " ]" << '\n';
}
}

Expand Down Expand Up @@ -310,13 +310,13 @@ void summary_perf(const Graph& graph) {
return;
}

std::cout << "======= ENABLE_DEBUG_CAPS:OV_CPU_SUMMARY_PERF ======" << std::endl;
std::cout << "======= ENABLE_DEBUG_CAPS:OV_CPU_SUMMARY_PERF ======" << '\n';
std::cout << "Summary of " << graph.GetName() << " @" << std::hash<uint64_t>{}(reinterpret_cast<uint64_t>(&graph))
<< std::endl;
std::cout << " Total(us): " << (uint64_t)(total) << std::endl;
std::cout << " Total_avg(us): " << (uint64_t)(total_avg) << std::endl;
<< '\n';
std::cout << " Total(us): " << (uint64_t)(total) << '\n';
std::cout << " Total_avg(us): " << (uint64_t)(total_avg) << '\n';
{
std::cout << " perf_by_type:" << std::endl;
std::cout << " perf_by_type:" << '\n';
std::vector<std::pair<std::string, double>> A;
A.reserve(perf_by_type.size());
for (auto& it : perf_by_type) {
Expand All @@ -333,12 +333,12 @@ void summary_perf(const Graph& graph) {
break;
}
ss << std::setw(10) << std::right << percentage << " % : " << std::setw(8) << std::right << it.second
<< "(us) " << it.first << std::endl;
<< "(us) " << it.first << '\n';
std::cout << ss.str();
}
}
{
std::cout << " perf_by_node:" << std::endl;
std::cout << " perf_by_node:" << '\n';
std::vector<std::pair<NodePtr, double>> A;
A.reserve(perf_by_node.size());
for (auto& it : perf_by_node) {
Expand All @@ -361,7 +361,7 @@ void summary_perf(const Graph& graph) {
ss << std::setw(10) << std::right << std::fixed << std::setprecision(2) << percentage << " % "
<< std::setw(8) << std::right << node->PerfCounter().avg() << "(us)x" << node->PerfCounter().count()
<< " #" << node->getExecIndex() << " " << node->getName() << " "
<< node->getTypeStr() + "_" + node->getPrimitiveDescriptorType() << std::endl;
<< node->getTypeStr() + "_" + node->getPrimitiveDescriptorType() << '\n';
std::cout << ss.str();
}
}
Expand Down
4 changes: 2 additions & 2 deletions src/plugins/intel_cpu/src/node.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2090,8 +2090,8 @@ int Node::inPlaceOutPort(int portIdx) const {
}

void Node::resolveInPlaceDirection() {
enum InplaceDirectionType { UP, DOWN, CYCLIC, NONE };
enum PortType { INPUT, OUTPUT };
enum InplaceDirectionType : uint8_t { UP, DOWN, CYCLIC, NONE };
enum PortType : uint8_t { INPUT, OUTPUT };

auto inPlaceDirection = [](const Node* node, PortType portType, int portNum) -> InplaceDirectionType {
if (PortType::INPUT == portType) {
Expand Down
2 changes: 1 addition & 1 deletion src/plugins/intel_cpu/src/nodes/common/cpu_convert.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ using namespace dnnl::impl::utils;
using namespace dnnl::impl::cpu::x64;
using namespace Xbyak;

enum f8_type { none, f8e4m3, f8e5m2 };
enum f8_type : uint8_t { none, f8e4m3, f8e5m2 };

template <typename src_t, typename dst_t>
f8_type get_f8_type() {
Expand Down
2 changes: 1 addition & 1 deletion src/plugins/intel_cpu/src/nodes/eltwise.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2607,7 +2607,7 @@ void Eltwise::initSupportedPrimitiveDescriptors() {
}
}

enum LayoutType { Planar, ChannelsFirst, Blocked };
enum LayoutType : uint8_t { Planar, ChannelsFirst, Blocked };

auto initDesc = [&](LayoutType lt, const bool useEltwiseExecutor = false, const bool useJit = false) -> NodeDesc {
auto createMemoryDesc =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ void SubgraphExecutor::segfault_detector() {
__sighandler_t signal_handler = [](int signal) {
std::lock_guard<std::mutex> guard(err_print_lock);
if (auto segfault_detector_emitter = ov::intel_cpu::g_custom_segfault_handler->local()) {
std::cout << segfault_detector_emitter->info() << std::endl;
std::cout << segfault_detector_emitter->info() << '\n';
}
auto tid = parallel_get_thread_num();
OPENVINO_THROW("Segfault was caught by the signal handler in subgraph node execution on thread " +
Expand Down
15 changes: 13 additions & 2 deletions src/plugins/intel_cpu/src/nodes/mvn.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -233,7 +233,18 @@ struct jit_uni_mvn_mean_variance_kernel_f32 : public jit_uni_mvn_mean_variance_k

size_t src_stride = 0;

enum { VECTOR, TAIL8, TAIL4, TAIL2, TAIL1, TAIL8_FILL, TAIL4_FILL, TAIL2_FILL, TAIL1_FILL, LOAD_EMITTERS_NUM };
enum : uint8_t {
VECTOR,
TAIL8,
TAIL4,
TAIL2,
TAIL1,
TAIL8_FILL,
TAIL4_FILL,
TAIL2_FILL,
TAIL1_FILL,
LOAD_EMITTERS_NUM
};
std::unique_ptr<jit_load_emitter> load_emitter[LOAD_EMITTERS_NUM];
std::vector<size_t> load_pool_gpr_idxs;

Expand Down Expand Up @@ -1106,7 +1117,7 @@ struct jit_uni_mvn_kernel_f32 : public jit_uni_mvn_kernel, public jit_generator
Vmm vmm_d_weights = Vmm(0);
Vmm vmm_d_bias = Vmm(1);

enum { VECTOR, TAIL8, TAIL4, TAIL2, TAIL1, EMITTERS_NUM };
enum : uint8_t { VECTOR, TAIL8, TAIL4, TAIL2, TAIL1, EMITTERS_NUM };
std::unique_ptr<jit_load_emitter> load_emitter[EMITTERS_NUM];
std::unique_ptr<jit_store_emitter> store_emitter[EMITTERS_NUM];
std::vector<size_t> store_pool_gpr_idxs;
Expand Down
2 changes: 1 addition & 1 deletion src/plugins/intel_cpu/src/nodes/subgraph.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -223,7 +223,7 @@ void Subgraph::initSupportedPrimitiveDescriptors() {
}
#endif

enum LayoutType { Planar, ChannelsFirst, Blocked };
enum LayoutType : uint8_t { Planar, ChannelsFirst, Blocked };
auto initDesc = [&](LayoutType lt) -> NodeDesc {
auto createMemoryDesc =
[lt](const Shape& shape, ov::element::Type prc, size_t offset) -> std::shared_ptr<CpuBlockedMemoryDesc> {
Expand Down
22 changes: 11 additions & 11 deletions src/plugins/intel_cpu/src/utils/blob_dump.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -175,79 +175,79 @@ void BlobDumper::dumpAsTxt(std::ostream& stream) const {
stream << d << " ";
}
stream << "(" << data_size << ")"
<< " by address 0x" << std::hex << memory->getDataAs<const int64_t>() << std::dec << std::endl;
<< " by address 0x" << std::hex << memory->getDataAs<const int64_t>() << std::dec << '\n';

const void* ptr = memory->getData();

switch (desc.getPrecision()) {
case ov::element::f32: {
auto* blob_ptr = reinterpret_cast<const float*>(ptr);
for (size_t i = 0; i < data_size; i++) {
stream << blob_ptr[desc.getElementOffset(i)] << std::endl;
stream << blob_ptr[desc.getElementOffset(i)] << '\n';
}
break;
}
case ov::element::i32: {
auto* blob_ptr = reinterpret_cast<const int32_t*>(ptr);
for (size_t i = 0; i < data_size; i++) {
stream << blob_ptr[desc.getElementOffset(i)] << std::endl;
stream << blob_ptr[desc.getElementOffset(i)] << '\n';
}
break;
}
case ov::element::bf16: {
auto* blob_ptr = reinterpret_cast<const bfloat16_t*>(ptr);
for (size_t i = 0; i < data_size; i++) {
float fn = static_cast<float>(blob_ptr[desc.getElementOffset(i)]);
stream << fn << std::endl;
stream << fn << '\n';
}
break;
}
case ov::element::f16: {
auto* blob_ptr = reinterpret_cast<const float16*>(ptr);
for (size_t i = 0; i < data_size; i++) {
stream << blob_ptr[desc.getElementOffset(i)] << std::endl;
stream << blob_ptr[desc.getElementOffset(i)] << '\n';
}
break;
}
case ov::element::i8: {
auto* blob_ptr = reinterpret_cast<const int8_t*>(ptr);
for (size_t i = 0; i < data_size; i++) {
stream << static_cast<int>(blob_ptr[desc.getElementOffset(i)]) << std::endl;
stream << static_cast<int>(blob_ptr[desc.getElementOffset(i)]) << '\n';
}
break;
}
case ov::element::u8: {
auto* blob_ptr = reinterpret_cast<const uint8_t*>(ptr);
for (size_t i = 0; i < data_size; i++) {
stream << static_cast<int>(blob_ptr[desc.getElementOffset(i)]) << std::endl;
stream << static_cast<int>(blob_ptr[desc.getElementOffset(i)]) << '\n';
}
break;
}
case ov::element::i64: {
auto* blob_ptr = reinterpret_cast<const int64_t*>(ptr);
for (size_t i = 0; i < data_size; i++) {
stream << blob_ptr[desc.getElementOffset(i)] << std::endl;
stream << blob_ptr[desc.getElementOffset(i)] << '\n';
}
break;
}
case ov::element::u32: {
auto* blob_ptr = reinterpret_cast<const uint32_t*>(ptr);
for (size_t i = 0; i < data_size; i++) {
stream << blob_ptr[desc.getElementOffset(i)] << std::endl;
stream << blob_ptr[desc.getElementOffset(i)] << '\n';
}
break;
}
case ov::element::u16: {
auto* blob_ptr = reinterpret_cast<const uint16_t*>(ptr);
for (size_t i = 0; i < data_size; i++) {
stream << blob_ptr[desc.getElementOffset(i)] << std::endl;
stream << blob_ptr[desc.getElementOffset(i)] << '\n';
}
break;
}
case ov::element::i16: {
auto* blob_ptr = reinterpret_cast<const int16_t*>(ptr);
for (size_t i = 0; i < data_size; i++) {
stream << blob_ptr[desc.getElementOffset(i)] << std::endl;
stream << blob_ptr[desc.getElementOffset(i)] << '\n';
}
break;
}
Expand Down
12 changes: 6 additions & 6 deletions src/plugins/intel_cpu/src/utils/debug_capabilities.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ void DebugLogEnabled::break_at(const std::string& log) {
static const char* p_brk = std::getenv("OV_CPU_DEBUG_LOG_BRK");
if (p_brk && log.find(p_brk) != std::string::npos) {
std::cout << "[ DEBUG ] "
<< " Debug log breakpoint hit" << std::endl;
<< " Debug log breakpoint hit" << '\n';
# if defined(_MSC_VER)
__debugbreak();
# elif defined(__APPLE__) || defined(OPENVINO_ARCH_ARM) || defined(OPENVINO_ARCH_ARM64) || \
Expand Down Expand Up @@ -406,11 +406,11 @@ std::ostream& operator<<(std::ostream& os, const Shape& shape) {

// Print complex data structures in a textualized form to the console is an efficient way to investigate them
std::ostream& operator<<(std::ostream& os, const Graph& g) {
os << "ov::intel_cpu::Graph " << g.GetName() << " {" << std::endl;
os << "ov::intel_cpu::Graph " << g.GetName() << " {" << '\n';
for (auto& graphNode : g.GetNodes()) {
std::cout << *graphNode << std::endl;
std::cout << *graphNode << '\n';
}
os << "};" << std::endl;
os << "};" << '\n';
return os;
}

Expand Down Expand Up @@ -556,13 +556,13 @@ std::ostream& operator<<(std::ostream& os, const PrintableModel& model) {

os << ") \t attrs:";
op->visit_attributes(osvis);
os << std::endl;
os << '\n';

// recursively output subgraphs
if (auto msubgraph = ov::as_type_ptr<op::util::MultiSubGraphOp>(op)) {
auto cnt = msubgraph->get_internal_subgraphs_size();
for (size_t i = 0; i < cnt; i++) {
os << "\t\t MultiSubGraphOp " << tag << msubgraph->get_friendly_name() << "[" << i << "]" << std::endl;
os << "\t\t MultiSubGraphOp " << tag << msubgraph->get_friendly_name() << "[" << i << "]" << '\n';
os << PrintableModel(*msubgraph->get_function(i).get(), tag, prefix + "\t\t");
}
}
Expand Down
14 changes: 7 additions & 7 deletions src/plugins/intel_cpu/src/utils/debug_capabilities.h
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,7 @@ static inline std::ostream& _write_all_to_stream(std::ostream& os, const T& arg,
if (DEBUG_ENABLE_NAME) { \
::std::stringstream ss___; \
ov::intel_cpu::_write_all_to_stream(ss___, prefix, DEBUG_ENABLE_NAME.get_tag(), " ", __VA_ARGS__); \
ostream << ss___.str() << std::endl; \
ostream << ss___.str() << '\n'; \
DEBUG_ENABLE_NAME.break_at(ss___.str()); \
} \
} while (0)
Expand Down Expand Up @@ -226,23 +226,23 @@ struct EnforceInferPrcDebug {
~EnforceInferPrcDebug() {
if (pattern_verbose) {
if (str_pos_pattern)
std::cout << "OV_CPU_INFER_PRC_POS_PATTERN=\"" << str_pos_pattern << "\"" << std::endl;
std::cout << "OV_CPU_INFER_PRC_POS_PATTERN=\"" << str_pos_pattern << "\"" << '\n';
if (str_neg_pattern)
std::cout << "OV_CPU_INFER_PRC_NEG_PATTERN=\"" << str_neg_pattern << "\"" << std::endl;
std::cout << "OV_CPU_INFER_PRC_NEG_PATTERN=\"" << str_neg_pattern << "\"" << '\n';
std::cout << "infer precision enforced Types: ";
size_t total_cnt = 0;
for (auto& ent : all_enabled_nodes) {
std::cout << ent.first << ",";
total_cnt += ent.second.size();
}
std::cout << " total number of nodes: " << total_cnt << std::endl;
std::cout << " total number of nodes: " << total_cnt << '\n';
for (auto& ent : all_enabled_nodes) {
std::cout << ent.first << " : " << std::endl;
std::cout << ent.first << " : " << '\n';
for (auto& name : ent.second) {
std::cout << "\t" << name << std::endl;
std::cout << "\t" << name << '\n';
}
}
std::cout << std::endl;
std::cout << '\n';
}
}

Expand Down
4 changes: 2 additions & 2 deletions src/plugins/intel_cpu/src/utils/node_dumper.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@ void dumpInputBlobs(const NodePtr& node, const DebugCapsConfig& config, int coun

std::string dump_file = createDumpFilePath(config.blobDumpDir, file_name, node->getExecIndex());

std::cout << "Dump inputs: " << dump_file << std::endl;
std::cout << "Dump inputs: " << dump_file << '\n';

auto& desc = prEdge->getMemory().getDesc();
if (desc.getPrecision() == ov::element::u1) {
Expand Down Expand Up @@ -190,7 +190,7 @@ void dumpOutputBlobs(const NodePtr& node, const DebugCapsConfig& config, int cou

std::string dump_file = createDumpFilePath(config.blobDumpDir, file_name, node->getExecIndex());

std::cout << "Dump outputs: " << dump_file << std::endl;
std::cout << "Dump outputs: " << dump_file << '\n';

auto& desc = childEdge->getMemory().getDesc();
if (desc.getPrecision() == ov::element::u1) {
Expand Down
Loading
Loading