Skip to content

Commit

Permalink
[CPU] Fix google-default-arguments clang-tidy remarks (openvinotoolki…
Browse files Browse the repository at this point in the history
…t#28772)

### Details:
 - Fix "google-default-arguments" issues reported by clang-tidy
 - Enable "google-default-arguments" clang-tidy checks on CI by default
- Introduce "emit_code_impl" in JIT emitters to avoid default arguments
in virtual methods

### Tickets:
 - N/A
  • Loading branch information
aobolensk authored Feb 12, 2025
1 parent 3c2526a commit d04fd6e
Show file tree
Hide file tree
Showing 45 changed files with 192 additions and 157 deletions.
29 changes: 24 additions & 5 deletions src/common/snippets/include/snippets/emitter.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -52,16 +52,21 @@ class Emitter {

/**
* @brief called by generator to generate code to produce target code for a specific operation
* @details
* Avoid passing default arguments to virtual function, but still allow user to call
* emit_code function without "pool" or "gpr"
* @param in vector of vector argument registers
* @param out vector of vector resulting registers
* @param pool optional vector of free vector registers which might be used inside method
* @param gpr vector of free generam puproce registers which might be used inside method
* @param gpr vector of free general purpose registers which might be used inside method
* @return void
*/
virtual void emit_code(const std::vector<size_t>& in,
const std::vector<size_t>& out,
const std::vector<size_t>& pool = {},
const std::vector<size_t>& gpr = {}) const = 0;
void emit_code(const std::vector<size_t>& in,
const std::vector<size_t>& out,
const std::vector<size_t>& pool = {},
const std::vector<size_t>& gpr = {}) const {
emit_code_impl(in, out, pool, gpr);
}

/**
* @brief called by generator to generate data section, if needed for a specific operation
Expand All @@ -70,6 +75,20 @@ class Emitter {
virtual void emit_data() const {}

virtual ~Emitter() = default;

private:
/**
* @brief called by generator to generate code to produce target code for a specific operation
* @param in vector of vector argument registers
* @param out vector of vector resulting registers
* @param pool optional vector of free vector registers which might be used inside method
* @param gpr vector of free general purpose registers which might be used inside method
* @return void
*/
virtual void emit_code_impl(const std::vector<size_t>& in,
const std::vector<size_t>& out,
const std::vector<size_t>& pool,
const std::vector<size_t>& gpr) const = 0;
};

} // namespace snippets
Expand Down
9 changes: 5 additions & 4 deletions src/common/snippets/tests/include/lowering_utils.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,12 @@ using BlockedShapeVector = ov::snippets::op::Subgraph::BlockedShapeVector;
class DummyEmitter : public ov::snippets::Emitter {
public:
DummyEmitter(const std::vector<ov::Node::type_info_t>& custom_opset = {}) : ov::snippets::Emitter() {}
void emit_code(const std::vector<size_t>&,
const std::vector<size_t>&,
const std::vector<size_t>&,
const std::vector<size_t>&) const override {}
void emit_data() const override {}
protected:
void emit_code_impl(const std::vector<size_t>&,
const std::vector<size_t>&,
const std::vector<size_t>&,
const std::vector<size_t>&) const override {}
};

struct DummyCompiledSnippet : public ov::snippets::CompiledSnippet {
Expand Down
1 change: 0 additions & 1 deletion src/plugins/intel_cpu/src/.clang-tidy
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,6 @@ Checks: >
-cppcoreguidelines-narrowing-conversions,
-cppcoreguidelines-pro-bounds-pointer-arithmetic,
-google-build-using-namespace,
-google-default-arguments,
-google-explicit-constructor,
-google-readability-casting,
-google-readability-todo,
Expand Down
8 changes: 4 additions & 4 deletions src/plugins/intel_cpu/src/cpu_memory.h
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,7 @@ class IMemory {
// Caution!!! This action invalidates the previous data layout. The old data may become unreachable.
virtual void redefineDesc(MemoryDescPtr desc) = 0;

virtual void load(const IMemory& src, bool ftz = true) const = 0;
virtual void load(const IMemory& src, bool ftz) const = 0;

virtual MemoryBlockPtr getMemoryBlock() const = 0;

Expand Down Expand Up @@ -260,7 +260,7 @@ class StaticMemory final : public IMemory {
// Always throws since a static memory descriptor should not be modified
void redefineDesc(MemoryDescPtr desc) override;

void load(const IMemory& src, bool ftz = true) const override;
void load(const IMemory& src, bool ftz) const override;

MemoryBlockPtr getMemoryBlock() const override;

Expand Down Expand Up @@ -315,7 +315,7 @@ class Memory : public IMemory {

void redefineDesc(MemoryDescPtr desc) override;

void load(const IMemory& src, bool ftz = true) const override;
void load(const IMemory& src, bool ftz) const override;
void nullify() override;

dnnl::engine getEngine() const {
Expand Down Expand Up @@ -421,7 +421,7 @@ class StringMemory : public IMemory {

void redefineDesc(MemoryDescPtr desc) override;

void load(const IMemory& src, bool ftz = false) const override;
void load(const IMemory& src, bool ftz) const override;

MemoryBlockPtr getMemoryBlock() const override;

Expand Down
2 changes: 1 addition & 1 deletion src/plugins/intel_cpu/src/cpu_tensor.h
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ class Tensor : public ITensor {

const ov::Strides& get_strides() const override;

void* data(const element::Type& type = {}) const override;
void* data(const element::Type& type) const override;

MemoryPtr get_memory() {
return m_memptr;
Expand Down
2 changes: 1 addition & 1 deletion src/plugins/intel_cpu/src/dnnl_postops_composer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -659,7 +659,7 @@ static MemoryPtr prepackDecompressionParams(const MemoryCPtr& paramsPtr,
srcFormat);
auto srcMem = std::make_shared<Memory>(engine, srcMemoryDesc, paramsPtr->getData());

dstMem->load(*srcMem);
dstMem->load(*srcMem, true);
return dstMem;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,10 +43,10 @@ const std::vector<size_t> jit_emitter::store_gpr_regs = {
static const std::vector<size_t> vec_regs = {0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15,
16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31};

void jit_emitter::emit_code(const std::vector<size_t>& in_idxs,
const std::vector<size_t>& out_idxs,
const std::vector<size_t>& pool_vec_idxs,
const std::vector<size_t>& pool_gpr_idxs) const {
void jit_emitter::emit_code_impl(const std::vector<size_t>& in_idxs,
const std::vector<size_t>& out_idxs,
const std::vector<size_t>& pool_vec_idxs,
const std::vector<size_t>& pool_gpr_idxs) const {
emitter_preamble(in_idxs, out_idxs, pool_vec_idxs, pool_gpr_idxs);

emit_impl(in_idxs, out_idxs);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,11 +48,6 @@ class jit_emitter : public ov::snippets::Emitter {
p_table(0),
l_table(new Xbyak_aarch64::Label()) {}

void emit_code(const std::vector<size_t>& in_idxs,
const std::vector<size_t>& out_idxs,
const std::vector<size_t>& pool_vec_idxs = {},
const std::vector<size_t>& pool_gpr_idxs = {}) const override;

void emit_data() const override;

virtual size_t get_inputs_count() const = 0;
Expand Down Expand Up @@ -84,6 +79,11 @@ class jit_emitter : public ov::snippets::Emitter {
virtual void prepare_table();
virtual void register_table_entries() {}

void emit_code_impl(const std::vector<size_t>& in_idxs,
const std::vector<size_t>& out_idxs,
const std::vector<size_t>& pool_vec_idxs,
const std::vector<size_t>& pool_gpr_idxs) const override;

void load_table_addr() const {
h->adr(p_table, *l_table.get());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,10 +65,10 @@ size_t jit_dnnl_emitter::get_inputs_num() const {
return 1;
}

void jit_dnnl_emitter::emit_code(const std::vector<size_t>& in_vec_idxs,
const std::vector<size_t>& out_vec_idxs,
const std::vector<size_t>& pool_vec_idxs,
const std::vector<size_t>& pool_gpr_idxs) const {
void jit_dnnl_emitter::emit_code_impl(const std::vector<size_t>& in_vec_idxs,
const std::vector<size_t>& out_vec_idxs,
const std::vector<size_t>& pool_vec_idxs,
const std::vector<size_t>& pool_gpr_idxs) const {
if (host_isa_ == cpu::x64::sse41) {
if (out_vec_idxs[0] != in_vec_idxs[0]) {
h->uni_vmovups(Xmm(out_vec_idxs[0]), Xmm(in_vec_idxs[0]));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,6 @@ namespace ov::intel_cpu {

class jit_dnnl_emitter : public jit_emitter {
public:
void emit_code(const std::vector<size_t>& in_vec_idxs,
const std::vector<size_t>& out_vec_idxs,
const std::vector<size_t>& pool_vec_idxs,
const std::vector<size_t>& pool_gpr_idxs) const override;

void emit_data() const override;

void emit_impl(const std::vector<size_t>& in_idxs, const std::vector<size_t>& out_idxs) const override{};
Expand All @@ -37,6 +32,11 @@ class jit_dnnl_emitter : public jit_emitter {
ov::element::Type exec_prc = ov::element::f32);
void set_injector();

void emit_code_impl(const std::vector<size_t>& in_vec_idxs,
const std::vector<size_t>& out_vec_idxs,
const std::vector<size_t>& pool_vec_idxs,
const std::vector<size_t>& pool_gpr_idxs) const override;

dnnl_alg_kind_t kind{dnnl_alg_kind_undef};
float alpha{0.f};
float beta{0.f};
Expand Down
8 changes: 4 additions & 4 deletions src/plugins/intel_cpu/src/emitters/plugin/x64/jit_emitter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -247,10 +247,10 @@ void jit_emitter::prepare_table() {
}
}

void jit_emitter::emit_code(const std::vector<size_t>& in_idxs,
const std::vector<size_t>& out_idxs,
const std::vector<size_t>& pool_vec_idxs,
const std::vector<size_t>& pool_gpr_idxs) const {
void jit_emitter::emit_code_impl(const std::vector<size_t>& in_idxs,
const std::vector<size_t>& out_idxs,
const std::vector<size_t>& pool_vec_idxs,
const std::vector<size_t>& pool_gpr_idxs) const {
emitter_preamble(in_idxs, out_idxs, pool_vec_idxs, pool_gpr_idxs);

emit_impl(in_idxs, out_idxs);
Expand Down
9 changes: 5 additions & 4 deletions src/plugins/intel_cpu/src/emitters/plugin/x64/jit_emitter.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -46,10 +46,6 @@ class jit_emitter : public ov::snippets::Emitter {
k_mask = Xbyak::Opmask(1); // FIXME: in general case we need preserve k_mask state as well
}

void emit_code(const std::vector<size_t>& in_idxs,
const std::vector<size_t>& out_idxs,
const std::vector<size_t>& pool_vec_idxs = {},
const std::vector<size_t>& pool_gpr_idxs = {}) const override;
void emit_data() const override;

virtual size_t get_inputs_num() const = 0;
Expand Down Expand Up @@ -84,6 +80,11 @@ class jit_emitter : public ov::snippets::Emitter {
ov::element::Type exec_prc_;
Xbyak::Opmask k_mask;

void emit_code_impl(const std::vector<size_t>& in_idxs,
const std::vector<size_t>& out_idxs,
const std::vector<size_t>& pool_vec_idxs,
const std::vector<size_t>& pool_gpr_idxs) const override;

virtual void prepare_table();
virtual void register_table_entries() {}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -101,10 +101,10 @@ jit_kernel_emitter::jit_kernel_emitter(jit_generator* h,
data_ptr_regs_idx = snippets::utils::transform_snippets_regs_to_idxs(data_ptr_regs, snippets::RegType::gpr);
}

void jit_kernel_emitter::emit_code(const std::vector<size_t>& in,
const std::vector<size_t>& out,
const std::vector<size_t>& pool_vec_idxs,
const std::vector<size_t>& pool_gpr_idxs) const {
void jit_kernel_emitter::emit_code_impl(const std::vector<size_t>& in,
const std::vector<size_t>& out,
const std::vector<size_t>& pool_vec_idxs,
const std::vector<size_t>& pool_gpr_idxs) const {
validate_arguments(in, out);
aux_vec_idxs = pool_vec_idxs;
aux_gpr_idxs = pool_gpr_idxs;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,10 +36,6 @@ class jit_kernel_emitter : public jit_emitter {
size_t get_inputs_count() const override {
return 0;
}
void emit_code(const std::vector<size_t>& in_idxs,
const std::vector<size_t>& out_idxs,
const std::vector<size_t>& pool_vec_idxs,
const std::vector<size_t>& pool_gpr_idxs) const override;

protected:
void validate_arguments(const std::vector<size_t>& in, const std::vector<size_t>& out) const override;
Expand All @@ -54,6 +50,11 @@ class jit_kernel_emitter : public jit_emitter {
virtual void init_data_pointers(const std::vector<Xbyak_aarch64::XReg>& arg_regs,
const std::vector<Xbyak_aarch64::XReg>& data_ptr_regs) const = 0;

void emit_code_impl(const std::vector<size_t>& in_idxs,
const std::vector<size_t>& out_idxs,
const std::vector<size_t>& pool_vec_idxs,
const std::vector<size_t>& pool_gpr_idxs) const override;

void emit_impl(const std::vector<size_t>& in, const std::vector<size_t>& out) const override;

jit_snippets_compile_args jcp;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,10 +39,10 @@ void jit_loop_begin_emitter::validate_arguments(const std::vector<size_t>& in, c
OV_CPU_JIT_EMITTER_ASSERT(loop_begin_label != nullptr, "has not inited label!");
}

void jit_loop_begin_emitter::emit_code(const std::vector<size_t>& in,
const std::vector<size_t>& out,
const std::vector<size_t>& pool_vec_idxs,
const std::vector<size_t>& pool_gpr_idxs) const {
void jit_loop_begin_emitter::emit_code_impl(const std::vector<size_t>& in,
const std::vector<size_t>& out,
const std::vector<size_t>& pool_vec_idxs,
const std::vector<size_t>& pool_gpr_idxs) const {
validate_arguments(in, out);
emit_impl(in, out);
}
Expand Down Expand Up @@ -123,7 +123,7 @@ void jit_loop_end_emitter::validate_arguments(const std::vector<size_t>& in, con
OV_CPU_JIT_EMITTER_ASSERT(loop_begin_label != nullptr, "has not inited begin label!");
}

void jit_loop_end_emitter::emit_code(const std::vector<size_t>& in,
void jit_loop_end_emitter::emit_code_impl(const std::vector<size_t>& in,
const std::vector<size_t>& out,
const std::vector<size_t>& pool_vec_idxs,
const std::vector<size_t>& pool_gpr_idxs) const {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,6 @@ class jit_loop_begin_emitter : public jit_emitter {
size_t get_inputs_count() const override {
return 0;
}

void emit_code(const std::vector<size_t>& in_idxs,
const std::vector<size_t>& out_idxs,
const std::vector<size_t>& pool_vec_idxs = {},
const std::vector<size_t>& pool_gpr_idxs = {}) const override;

std::shared_ptr<const Xbyak_aarch64::Label> get_begin_label() {
return loop_begin_label;
}
Expand All @@ -34,6 +28,11 @@ class jit_loop_begin_emitter : public jit_emitter {
void validate_arguments(const std::vector<size_t>& in, const std::vector<size_t>& out) const override;
void emit_impl(const std::vector<size_t>& in, const std::vector<size_t>& out) const override;

void emit_code_impl(const std::vector<size_t>& in_idxs,
const std::vector<size_t>& out_idxs,
const std::vector<size_t>& pool_vec_idxs,
const std::vector<size_t>& pool_gpr_idxs) const override;

std::shared_ptr<Xbyak_aarch64::Label> loop_begin_label;
size_t work_amount = 0;
int64_t wa_increment = 0;
Expand All @@ -54,10 +53,10 @@ class jit_loop_end_emitter : public jit_emitter {
return 0;
}

void emit_code(const std::vector<size_t>& in_idxs,
void emit_code_impl(const std::vector<size_t>& in_idxs,
const std::vector<size_t>& out_idxs,
const std::vector<size_t>& pool_vec_idxs = {},
const std::vector<size_t>& pool_gpr_idxs = {}) const override;
const std::vector<size_t>& pool_vec_idxs,
const std::vector<size_t>& pool_gpr_idxs) const override;

protected:
void validate_arguments(const std::vector<size_t>& in, const std::vector<size_t>& out) const override;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,10 +59,10 @@ void jit_debug_emitter::emit_impl(const std::vector<size_t>& in_idxs, const std:
m_target_emitter->emit_impl(in_idxs, out_idxs);
}

void jit_debug_emitter::emit_code(const std::vector<size_t>& in_idxs,
const std::vector<size_t>& out_idxs,
const std::vector<size_t>& pool_vec_idxs,
const std::vector<size_t>& pool_gpr_idxs) const {
void jit_debug_emitter::emit_code_impl(const std::vector<size_t>& in_idxs,
const std::vector<size_t>& out_idxs,
const std::vector<size_t>& pool_vec_idxs,
const std::vector<size_t>& pool_gpr_idxs) const {
if (m_decorator_emit_loc == EmissionLocation::preamble || m_decorator_emit_loc == EmissionLocation::both) {
m_decorator_emitter->emit_code(in_idxs, out_idxs, pool_vec_idxs, pool_gpr_idxs);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,6 @@ class jit_debug_emitter : public jit_emitter {
prepare_table();
}

void emit_code(const std::vector<size_t>& in_idxs,
const std::vector<size_t>& out_idxs,
const std::vector<size_t>& pool_vec_idxs = {},
const std::vector<size_t>& pool_gpr_idxs = {}) const override;
void emit_data() const override;

size_t get_inputs_num() const override;
Expand All @@ -45,6 +41,11 @@ class jit_debug_emitter : public jit_emitter {

void emit_impl(const std::vector<size_t>& in_idxs, const std::vector<size_t>& out_idxs) const override;

void emit_code_impl(const std::vector<size_t>& in_idxs,
const std::vector<size_t>& out_idxs,
const std::vector<size_t>& pool_vec_idxs,
const std::vector<size_t>& pool_gpr_idxs) const override;

void emitter_preamble(const std::vector<size_t>& in_idxs,
const std::vector<size_t>& out_idxs,
const std::vector<size_t>& pool_vec_idxs,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,10 +49,10 @@ jit_kernel_emitter::jit_kernel_emitter(jit_generator* h,
data_ptr_regs_idx = snippets::utils::transform_snippets_regs_to_idxs(data_ptr_regs, snippets::RegType::gpr);
}

void jit_kernel_emitter::emit_code(const std::vector<size_t>& in,
const std::vector<size_t>& out,
const std::vector<size_t>& pool_vec_idxs,
const std::vector<size_t>& pool_gpr_idxs) const {
void jit_kernel_emitter::emit_code_impl(const std::vector<size_t>& in,
const std::vector<size_t>& out,
const std::vector<size_t>& pool_vec_idxs,
const std::vector<size_t>& pool_gpr_idxs) const {
validate_arguments(in, out);
aux_vec_idxs = pool_vec_idxs;
aux_gpr_idxs = pool_gpr_idxs;
Expand Down
Loading

0 comments on commit d04fd6e

Please sign in to comment.