Skip to content

Commit

Permalink
Fixes as per comments
Browse files Browse the repository at this point in the history
  • Loading branch information
nshchego committed Aug 5, 2024
1 parent 5cd0426 commit e580c88
Show file tree
Hide file tree
Showing 9 changed files with 42 additions and 32 deletions.
12 changes: 12 additions & 0 deletions src/core/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,11 @@ target_include_directories(openvino_core_dev INTERFACE
$<BUILD_INTERFACE:${OpenVINO_SOURCE_DIR}/src/common/transformations/include>
$<BUILD_INTERFACE:${OpenVINO_SOURCE_DIR}/src/common/low_precision_transformations/include>)

target_include_directories(openvino_core_dev INTERFACE
$<BUILD_INTERFACE:$<$<TARGET_EXISTS:xbyak::xbyak>:$<TARGET_PROPERTY:xbyak::xbyak,INTERFACE_INCLUDE_DIRECTORIES>>>)

# ov_set_threading_interface_for(openvino_core_dev)

target_link_libraries(openvino_core_dev INTERFACE openvino::itt openvino::util)

set_target_properties(openvino_core_dev PROPERTIES EXPORT_NAME core::dev)
Expand Down Expand Up @@ -89,6 +94,8 @@ ov_build_target_faster(openvino_core_obj

ov_add_version_defines(src/version.cpp openvino_core_obj)

ov_set_threading_interface_for(openvino_core_dev)

target_link_libraries(openvino_core_obj PRIVATE openvino::reference openvino::util
openvino::pugixml openvino::shape_inference openvino::core::dev)

Expand All @@ -102,6 +109,8 @@ ov_ncc_naming_style(FOR_TARGET openvino_core_obj

ov_add_clang_format_target(openvino_core_clang FOR_SOURCES ${LIBRARY_SRC} ${PUBLIC_HEADERS} ${DEV_HEADERS})

target_compile_definitions(openvino_core_obj PRIVATE XBYAK_NO_OP_NAMES XBYAK64)

if(NOT BUILD_SHARED_LIBS)
target_compile_definitions(openvino_core_obj PUBLIC OPENVINO_STATIC_LIBRARY)
endif()
Expand Down Expand Up @@ -142,6 +151,9 @@ set_source_files_properties("${CMAKE_CURRENT_SOURCE_DIR}/src/pass/convert_precis
target_include_directories(openvino_core_obj PUBLIC $<BUILD_INTERFACE:${OV_CORE_INCLUDE_PATH}>
PRIVATE ${CMAKE_CURRENT_SOURCE_DIR}/src)

# target_include_directories(openvino_core_obj SYSTEM PRIVATE
# $<BUILD_INTERFACE:$<$<TARGET_EXISTS:xbyak::xbyak>:$<TARGET_PROPERTY:xbyak::xbyak,INTERFACE_INCLUDE_DIRECTORIES>>>)

#-----------------------------------------------------------------------------------------------
# Installation logic...
#-----------------------------------------------------------------------------------------------
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,11 @@
namespace ov {
namespace runtime {

/**
* @brief Computes the hash value for the input data
* @param src A pointer to the input data
* @param size The length of the input data in bytes
*/
size_t combine_hash(const void* src, size_t size);

} // namespace runtime
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -303,7 +303,6 @@ RegistersPool::Ptr RegistersPool::create(cpu_isa_t isa, std::initializer_list<Xb
default:
OPENVINO_THROW("Invalid isa argument in RegistersPool::create(): ", isa);
}
OPENVINO_THROW("Invalid isa argument in RegistersPool::create()");
#undef ISA_SWITCH_CASE
}

Expand Down
5 changes: 0 additions & 5 deletions src/core/reference/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,6 @@ ov_build_target_faster(${TARGET_NAME}

ov_set_threading_interface_for(${TARGET_NAME})

target_compile_definitions(${TARGET_NAME} PRIVATE XBYAK_NO_OP_NAMES XBYAK64)

if(NOT BUILD_SHARED_LIBS)
target_compile_definitions(${TARGET_NAME} PUBLIC OPENVINO_STATIC_LIBRARY)
endif()
Expand All @@ -38,9 +36,6 @@ target_include_directories(${TARGET_NAME} PUBLIC
$<BUILD_INTERFACE:${OV_CORE_DEV_API_PATH}>
$<BUILD_INTERFACE:${OV_CORE_INCLUDE_PATH}>)

target_include_directories(${TARGET_NAME} SYSTEM PRIVATE
$<BUILD_INTERFACE:$<$<TARGET_EXISTS:xbyak::xbyak>:$<TARGET_PROPERTY:xbyak::xbyak,INTERFACE_INCLUDE_DIRECTORIES>>>)

find_package(Threads REQUIRED)
target_link_libraries(${TARGET_NAME} PRIVATE Threads::Threads openvino::core::dev)

Expand Down
2 changes: 1 addition & 1 deletion src/core/reference/src/op/convert.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
#include "openvino/reference/convert.hpp"

#if defined(OPENVINO_ARCH_X86) || defined(OPENVINO_ARCH_X86_64)
# include "openvino/reference/utils/jit_generator.hpp"
# include "openvino/runtime/jit_generator.hpp"

using namespace ov::runtime;
#endif
Expand Down
2 changes: 1 addition & 1 deletion src/core/src/pass/serialize.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,8 @@
#include "openvino/opsets/opset1.hpp"
#include "openvino/pass/constant_folding.hpp"
#include "openvino/reference/convert.hpp"
#include "openvino/reference/utils/combine_hash.hpp"
#include "openvino/runtime/aligned_buffer.hpp"
#include "openvino/runtime/combine_hash.hpp"
#include "openvino/runtime/string_aligned_buffer.hpp"
#include "openvino/util/file_util.hpp"
#include "pugixml.hpp"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,10 @@

#include "openvino/core/visibility.hpp"
#include "openvino/core/parallel.hpp"
#include "openvino/reference/utils/combine_hash.hpp"
#include "openvino/runtime/combine_hash.hpp"

#if defined(OPENVINO_ARCH_X86) || defined(OPENVINO_ARCH_X86_64)
# include "openvino/reference/utils/registers_pool.hpp"
# include "openvino/runtime/registers_pool.hpp"
#endif // OPENVINO_ARCH_X86 || OPENVINO_ARCH_X86_64

#include <cstring>
Expand All @@ -23,21 +23,21 @@ namespace runtime {
namespace jit {

#define GET_OFF(field) offsetof(CombineHashCallArgs, field)
#define getReg64() RegistersPool::Reg<Xbyak::Reg64>(registersPool)
#define getVmm() RegistersPool::Reg<Vmm>(registersPool)
#define getXmm() RegistersPool::Reg<Xbyak::Xmm>(registersPool)
#define getReg64() RegistersPool::Reg<Xbyak::Reg64>(registers_pool)
#define getVmm() RegistersPool::Reg<Vmm>(registers_pool)
#define getXmm() RegistersPool::Reg<Xbyak::Xmm>(registers_pool)

struct CombineHashCompileParams {
};

struct CombineHashCallArgs {
const void* src_ptr;
void* dst_ptr;
uint64_t work_amount = 0lu;
const void* src_ptr = nullptr;
void* dst_ptr = nullptr;
uint64_t work_amount = 0lu;
uint64_t make_64_fold = 0lu;
};

typedef void (*fn_t)(const CombineHashCallArgs*);
typedef void (*hash_kernel)(const CombineHashCallArgs*);

template <cpu_isa_t isa>
class CombineHash : public Generator {
Expand All @@ -63,7 +63,7 @@ class CombineHash : public Generator {

void generate() {
this->preamble();
registersPool = RegistersPool::create(isa, {rax, rcx, rsp, rdi, k0});
registers_pool = RegistersPool::create(isa, {rax, rcx, rsp, rdi, k0});

r64_src = getReg64();
r64_dst = getReg64();
Expand All @@ -80,15 +80,15 @@ class CombineHash : public Generator {
restFold(v_dst);
tailFold(v_dst);

registersPool.reset();
registers_pool.reset();
this->postamble();
}

static fn_t get() {
static hash_kernel get() {
static const CombineHashCompileParams params;
static CombineHash<isa> kernel(params);

return (fn_t)kernel.getCode();
return (hash_kernel)kernel.getCode();
}

void fillRestWorkMask(const Xbyak::Opmask& k_dst_mask,
Expand All @@ -107,7 +107,7 @@ class CombineHash : public Generator {
kmovq(k_dst_mask, rOnes);
}

void partialLoad(const Xbyak::Xmm& xmm_dst,
void partial_load(const Xbyak::Xmm& xmm_dst,
const Xbyak::Address& src_addr,
const Xbyak::Reg64& r64_load_num) {
Xbyak::Label l_partial, l_end;
Expand All @@ -130,7 +130,7 @@ class CombineHash : public Generator {
L(l_end);
}

void partialLoad(const Xbyak::Ymm& ymm_dst,
void partial_load(const Xbyak::Ymm& ymm_dst,
const Xbyak::Address& src_addr,
const Xbyak::Reg64& r64_load_num) {
Xbyak::Label l_xmm, l_partial, l_end;
Expand Down Expand Up @@ -187,7 +187,7 @@ class CombineHash : public Generator {
bool is_vpclmulqdq = false;

CombineHashCompileParams m_jcp;
RegistersPool::Ptr registersPool;
RegistersPool::Ptr registers_pool;

RegistersPool::Reg<Xbyak::Reg64> r64_src;
RegistersPool::Reg<Xbyak::Reg64> r64_dst;
Expand Down Expand Up @@ -263,7 +263,7 @@ void CombineHash<avx512_core>::initVectors() {
auto xmm_dst = Xbyak::Xmm(v_dst.getIdx());
auto xmm_shuf_mask = Xbyak::Xmm(v_shuf_mask.getIdx());
auto xmm_aux = getXmm();
auto k_rest_mask = RegistersPool::Reg<Xbyak::Opmask>(registersPool);
auto k_rest_mask = RegistersPool::Reg<Xbyak::Opmask>(registers_pool);
// Initial CRC
mov(r64_aux, CRC_VAL);
vpxorq(v_dst, v_dst, v_dst);
Expand Down Expand Up @@ -297,13 +297,13 @@ void CombineHash<isa>::initVectors() {
auto xmm_dst = Xbyak::Xmm(v_dst.getIdx());
auto xmm_shuf_mask = Xbyak::Xmm(v_shuf_mask.getIdx());
auto xmm_aux = getXmm();
auto k_rest_mask = RegistersPool::Reg<Xbyak::Opmask>(registersPool);
auto k_rest_mask = RegistersPool::Reg<Xbyak::Opmask>(registers_pool);
// Initial CRC
mov(r64_aux, CRC_VAL);
vpxorq(v_dst, v_dst, v_dst);
vpinsrq(xmm_dst, xmm_dst, r64_aux, 0x1);
// First xor with source
partialLoad(xmm_aux, ptr[r64_src], r64_work_amount);
partial_load(xmm_aux, ptr[r64_src], r64_work_amount);
vpshufb(xmm_aux, xmm_aux, xmm_shuf_mask);
vpxorq(xmm_dst, xmm_dst, xmm_aux);
sub(r64_work_amount, xmm_len);
Expand Down Expand Up @@ -522,7 +522,7 @@ void CombineHash<avx512_core>::tailFold(const Vmm& v_dst) {
auto xmm_aux = getXmm();
auto xmm_aux_1 = getXmm();
auto xmm_aux_2 = getXmm();
auto k_rest_mask = RegistersPool::Reg<Xbyak::Opmask>(registersPool);
auto k_rest_mask = RegistersPool::Reg<Xbyak::Opmask>(registers_pool);

fillRestWorkMask(k_rest_mask, r64_work_amount);

Expand Down Expand Up @@ -591,7 +591,7 @@ const uint8_t CombineHash<isa>::SHUF_MASK[] = { 0b00001111, 0b00001110, 0b000011

size_t combine_hash(const void* src, size_t size) {
#if defined(OPENVINO_ARCH_X86) || defined(OPENVINO_ARCH_X86_64)
jit::fn_t kernel;
jit::hash_kernel kernel = nullptr;

if (jit::Generator::mayiuse(jit::avx512_core)) {
kernel = jit::CombineHash<jit::avx512_core>::get();
Expand All @@ -617,7 +617,6 @@ size_t combine_hash(const void* src, size_t size) {
}
uint64_t work_amount = (el_per_thread + start > size) ? size - start : el_per_thread;

size_t res = 0lu;
jit::CombineHashCallArgs args;

args.src_ptr = reinterpret_cast<const uint8_t *>(src) + start;
Expand Down Expand Up @@ -647,7 +646,7 @@ size_t combine_hash(const void* src, size_t size) {
#endif // OPENVINO_ARCH_X86 || OPENVINO_ARCH_X86_64

constexpr auto cel_size = sizeof(size_t);
auto seed = static_cast<size_t>(size);
auto seed = size;
const auto data = static_cast<const size_t*>(src);
const auto d_end = std::next(data, size / cel_size);
// The constant value used as a magic number has been
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
# endif
# include <xbyak/xbyak_util.h>

# include "openvino/reference/utils/jit_generator.hpp"
# include "openvino/runtime/jit_generator.hpp"
# include "openvino/core/type/bfloat16.hpp"
# include "openvino/core/type/float16.hpp"

Expand Down

0 comments on commit e580c88

Please sign in to comment.