Skip to content

[Outdated, see #13790][CPU] StackAllocator for JIT kernels #13132

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.

Sign up for GitHub

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

Closed
lohika-denis-kotov wants to merge 20 commits into from
Closed

[Outdated, see #13790][CPU] StackAllocator for JIT kernels #13132

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
5b10966
Initial implementation for StackAllocator
lohika-denis-kotov Sep 16, 2022
f5d563c
Created StackAllocatorTest
lohika-denis-kotov Sep 17, 2022
6a6aab3
Added StackAllocator::Reg<> for simplifing saving registers on stack
lohika-denis-kotov Sep 22, 2022
a6c148d
Added tests for new StackAllocator::Reg<> class
lohika-denis-kotov Sep 22, 2022
d4e4ce6
Added alignment for StackAllocator
lohika-denis-kotov Oct 7, 2022
092b739
Added saving data from stack to register
lohika-denis-kotov Oct 7, 2022
3b12678
Update tests for StackAllocator
lohika-denis-kotov Oct 8, 2022
937de86
Added Transaction class
lohika-denis-kotov Oct 9, 2022
a0ec185
Updated stack_allocator test for Transaction
lohika-denis-kotov Oct 9, 2022
28c56ad
Updated tests for StackAllocator for Transaction
lohika-denis-kotov Oct 9, 2022
e70e1a7
Made StackAllocator::Address not copiable and movable
lohika-denis-kotov Oct 9, 2022
7bf863c
Changed names of variables in tests for StackAllocator
lohika-denis-kotov Oct 9, 2022
5104f83
Fixed name of types for IsaParam
lohika-denis-kotov Oct 9, 2022
126f609
Added tests for StackAllocator::Reg<Xbyak::Xmm> and StackAllocator::R…
lohika-denis-kotov Oct 10, 2022
905c8e0
Renamed some tests to describe what it is doing
lohika-denis-kotov Oct 10, 2022
e95f4e5
Pass StackAllocator to Transaction, Address and Reg<> by reference
lohika-denis-kotov Oct 10, 2022
4de9d1a
Made StackAllocator::Address isInitialized() public and added explici…
lohika-denis-kotov Oct 13, 2022
ca20c13
Fix CI issue with redefinition of struct IsaParam
lohika-denis-kotov Oct 13, 2022
cc891cb
Remove using using namespace dnnl::impl::cpu; and changed it to alias
lohika-denis-kotov Nov 1, 2022
99650b4
Fix macos compile-time issues
lohika-denis-kotov Nov 2, 2022
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
11 changes: 9 additions & 2 deletions src/plugins/intel_cpu/src/nodes/kernels/registers_pool.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,12 @@
#include "ie_common.h"
#include "utils/cpu_utils.hpp"
#include <utility>
#include "stack_allocator.hpp"

namespace ov {
namespace intel_cpu {

using namespace dnnl::impl::cpu;
namespace x64 = dnnl::impl::cpu::x64;
Copy link
Copy Markdown
Contributor

@maxnick maxnick Nov 2, 2022

Choose a reason for hiding this comment

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

As far as I can see only x64::cpu_isa_t is used. M.b. it does make sense to use an alias for this type on the RegistersPool level:

class RegistersPool {
public:
    using cpu_isa_t = dnnl::impl::cpu::x64::cpu_isa_t;

So, within this header it can be accessed just as follows RegistersPool::cpu_isa_t
What do you think?

Copy link
Copy Markdown

@lohika-vmostovy lohika-vmostovy Nov 2, 2022

Choose a reason for hiding this comment

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

We also have other references to x64 namespace in this header like x64::cpu_isa_traits and cpu_isa_t enum members.

Copy link
Copy Markdown
Contributor

@maxnick maxnick Nov 2, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Do not see an elegant solution here, since the whole class is intended to be used with dnnl::impl::cpu::x64::cpu_isa_t members, and we do not have a possibility to import enumeration to the class (it has been introduced in c++20). I guess we should think about wrapping some dnnl entities on jit_base level.


/**
* The RegistersPool is the base class for the IsaRegistersPool template:
Expand Down Expand Up @@ -49,16 +50,22 @@ class RegistersPool {
Reg(const RegistersPool::Ptr& regPool) { initialize(regPool); }
Reg(const RegistersPool::Ptr& regPool, int requestedIdx) { initialize(regPool, requestedIdx); }
~Reg() { release(); }
Reg(Reg&& other) noexcept {
this->operator=(std::move(other));
}
Reg& operator=(Reg&& other) noexcept {
release();
reg = other.reg;
regPool = std::move(other.regPool);
return *this;
}
Reg(Reg&& other) noexcept : reg(other.reg), regPool(std::move(other.regPool)) {}
operator TReg&() { ensureValid(); return reg; }
operator const TReg&() const { ensureValid(); return reg; }
operator Xbyak::RegExp() const { ensureValid(); return reg; }
Reg& operator=(const StackAllocator::Address& addr) {
stack_mov(*this, addr);
return *this;
}
int getIdx() const { ensureValid(); return reg.getIdx(); }
friend Xbyak::RegExp operator+(const Reg& lhs, const Xbyak::RegExp& rhs) {
lhs.ensureValid();
Expand Down
Loading