From fa000dd315e0a12c972267b4ba9f0f980c7092f2 Mon Sep 17 00:00:00 2001 From: Mohammad Haghighipanah Date: Tue, 26 Nov 2024 10:18:16 -0800 Subject: [PATCH 1/4] fix coverity issue --- .../include/intel_gpu/primitives/resample.hpp | 2 +- .../src/graph/impls/onednn/convolution_onednn.cpp | 2 +- .../fully_connected_kernel_bf_tiled.cpp | 10 ++++++++++ .../src/plugin/transformations/kv_cache_fusion.cpp | 2 +- 4 files changed, 13 insertions(+), 3 deletions(-) diff --git a/src/plugins/intel_gpu/include/intel_gpu/primitives/resample.hpp b/src/plugins/intel_gpu/include/intel_gpu/primitives/resample.hpp index 62d32a3619e329..f6e32661974cb8 100644 --- a/src/plugins/intel_gpu/include/intel_gpu/primitives/resample.hpp +++ b/src/plugins/intel_gpu/include/intel_gpu/primitives/resample.hpp @@ -15,7 +15,7 @@ namespace cldnn { struct resample : public primitive_base { CLDNN_DECLARE_PRIMITIVE(resample) - resample() : primitive_base("", {}) {} + resample() : primitive_base("", {}), scales_port(0) {} using InterpolateOp = ov::op::util::InterpolateBase; diff --git a/src/plugins/intel_gpu/src/graph/impls/onednn/convolution_onednn.cpp b/src/plugins/intel_gpu/src/graph/impls/onednn/convolution_onednn.cpp index a11ceef8b0f2dd..a2d3c007e29aa8 100644 --- a/src/plugins/intel_gpu/src/graph/impls/onednn/convolution_onednn.cpp +++ b/src/plugins/intel_gpu/src/graph/impls/onednn/convolution_onednn.cpp @@ -121,7 +121,7 @@ struct convolution_onednn : typed_primitive_onednn_impl { private: int _zero_point_mask; - dnnl::memory::data_type _wzp_data_type; + dnnl::memory::data_type _wzp_data_type = dnnl::memory::data_type::undef; protected: std::unique_ptr clone() const override { diff --git a/src/plugins/intel_gpu/src/kernel_selector/kernels/fully_connected/fully_connected_kernel_bf_tiled.cpp b/src/plugins/intel_gpu/src/kernel_selector/kernels/fully_connected/fully_connected_kernel_bf_tiled.cpp index 02304512637783..b0f58d65693598 100644 --- a/src/plugins/intel_gpu/src/kernel_selector/kernels/fully_connected/fully_connected_kernel_bf_tiled.cpp +++ b/src/plugins/intel_gpu/src/kernel_selector/kernels/fully_connected/fully_connected_kernel_bf_tiled.cpp @@ -757,6 +757,11 @@ void FullyConnected_bf_tiled::GetUpdateDispatchDataFunc(KernelData& kd) const { const auto& prim_params = static_cast(params); size_t quantize_grp_size = get_dynamic_quantize_group_size(prim_params); + if (quantize_grp_size == 0) { + std::cerr << "Error: quantize_grp_size is zero." << std::endl; + return; + } + size_t output_batch = get_output_aligned_bf_size(prim_params, false).first; // Get index of the added shape-agnostic kernel @@ -941,6 +946,11 @@ KernelsData FullyConnected_bf_tiled::GetMultiKernelsData(const Params ¶ms, size_t quantize_grp_size = get_dynamic_quantize_group_size(fc_params); + if (quantize_grp_size == 0) { + std::cerr << "Error: quantize_grp_size is zero." << std::endl; + return KernelsData(); + } + bool bProperInput = fc_params.inputs[0].GetLayout() == dl; if (!bProperInput && !fc_params.inputs[0].PitchesDifferFromLogicalDims()) { bProperInput = (dl == DataLayout::fb && fc_params.inputs[0].GetLayout() == DataLayout::fyxb) || diff --git a/src/plugins/intel_gpu/src/plugin/transformations/kv_cache_fusion.cpp b/src/plugins/intel_gpu/src/plugin/transformations/kv_cache_fusion.cpp index b97389a7d18c76..8be42a1311094b 100644 --- a/src/plugins/intel_gpu/src/plugin/transformations/kv_cache_fusion.cpp +++ b/src/plugins/intel_gpu/src/plugin/transformations/kv_cache_fusion.cpp @@ -63,7 +63,7 @@ KVCacheFusionMatcher::KVCacheFusionMatcher() { return false; // TODO: Support conversion internally - if (concat_node->get_output_element_type(0) != past_node->get_output_element_type(0)) + if (!concat_node || concat_node->get_output_element_type(0) != past_node->get_output_element_type(0)) return false; auto variable = past_node->get_variable(); From 80c6ff90904e3c31f1db463c8b864644f299cf78 Mon Sep 17 00:00:00 2001 From: Mohammad Haghighipanah Date: Wed, 27 Nov 2024 11:25:15 -0800 Subject: [PATCH 2/4] addressing feedback --- .../fully_connected_kernel_bf_tiled.cpp | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/src/plugins/intel_gpu/src/kernel_selector/kernels/fully_connected/fully_connected_kernel_bf_tiled.cpp b/src/plugins/intel_gpu/src/kernel_selector/kernels/fully_connected/fully_connected_kernel_bf_tiled.cpp index b0f58d65693598..890f2ee7fb7619 100644 --- a/src/plugins/intel_gpu/src/kernel_selector/kernels/fully_connected/fully_connected_kernel_bf_tiled.cpp +++ b/src/plugins/intel_gpu/src/kernel_selector/kernels/fully_connected/fully_connected_kernel_bf_tiled.cpp @@ -757,10 +757,7 @@ void FullyConnected_bf_tiled::GetUpdateDispatchDataFunc(KernelData& kd) const { const auto& prim_params = static_cast(params); size_t quantize_grp_size = get_dynamic_quantize_group_size(prim_params); - if (quantize_grp_size == 0) { - std::cerr << "Error: quantize_grp_size is zero." << std::endl; - return; - } + OPENVINO_ASSERT(quantize_grp_size != 0, "Error: quantize_grp_size is zero."); size_t output_batch = get_output_aligned_bf_size(prim_params, false).first; @@ -945,11 +942,7 @@ KernelsData FullyConnected_bf_tiled::GetMultiKernelsData(const Params ¶ms, const auto& fc_params = static_cast(params); size_t quantize_grp_size = get_dynamic_quantize_group_size(fc_params); - - if (quantize_grp_size == 0) { - std::cerr << "Error: quantize_grp_size is zero." << std::endl; - return KernelsData(); - } + OPENVINO_ASSERT(quantize_grp_size != 0, "Error: quantize_grp_size is zero."); bool bProperInput = fc_params.inputs[0].GetLayout() == dl; if (!bProperInput && !fc_params.inputs[0].PitchesDifferFromLogicalDims()) { From cd4f7f3e0633ab3af17e86b0d18b870e20c6a0c3 Mon Sep 17 00:00:00 2001 From: Mohammad Haghighipanah Date: Thu, 5 Dec 2024 15:58:44 -0800 Subject: [PATCH 3/4] addressing coverity --- src/core/include/openvino/pass/pattern/matcher.hpp | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/src/core/include/openvino/pass/pattern/matcher.hpp b/src/core/include/openvino/pass/pattern/matcher.hpp index bbd7e32b0a1802..659c18be027b1a 100644 --- a/src/core/include/openvino/pass/pattern/matcher.hpp +++ b/src/core/include/openvino/pass/pattern/matcher.hpp @@ -62,10 +62,13 @@ class OPENVINO_API Matcher { // Avoid implicit string construction from nullptr. Matcher(const std::shared_ptr pattern_node, std::nullptr_t name) = delete; - Matcher() = default; - Matcher(Output& pattern_node) : m_pattern_node{pattern_node} {} + Matcher() + : m_match_root{}, m_pattern_node{}, m_pattern_map{}, m_pattern_value_maps{}, m_matched_list{}, m_name{""}, m_strict_mode{false} {} + Matcher(Output& pattern_node) + : m_match_root{}, m_pattern_node{pattern_node}, m_pattern_map{}, m_pattern_value_maps{}, m_matched_list{}, m_name{""}, m_strict_mode{false} {} - Matcher(Output& pattern_node, const std::string& name) : m_pattern_node(pattern_node), m_name{name} {} + Matcher(Output& pattern_node, const std::string& name) + : m_match_root{}, m_pattern_node{pattern_node}, m_pattern_map{}, m_pattern_value_maps{}, m_matched_list{}, m_name{name}, m_strict_mode{false} {} /// \brief Constructs a Matcher object /// @@ -73,9 +76,7 @@ class OPENVINO_API Matcher { /// \param name is a string which is used for logging and disabling a matcher /// \param strict_mode forces a matcher to consider shapes and ET of nodes Matcher(const Output& pattern_node, const std::string& name, bool strict_mode) - : m_pattern_node(pattern_node), - m_name(name), - m_strict_mode(strict_mode) {} + : m_match_root{}, m_pattern_node{pattern_node}, m_pattern_map{}, m_pattern_value_maps{}, m_matched_list{}, m_name{name}, m_strict_mode{strict_mode} {} // Some matches should start on a node rather than an output. These three constructors // are transition until we work out the right way to do that. From 92d769873edec35257cfae4a07f58e773b0bc06d Mon Sep 17 00:00:00 2001 From: Mohammad Haghighipanah Date: Fri, 6 Dec 2024 10:14:45 -0800 Subject: [PATCH 4/4] fixed format --- .../include/openvino/pass/pattern/matcher.hpp | 32 ++++++++++++++++--- .../fully_connected_kernel_bf_tiled.cpp | 4 +-- 2 files changed, 30 insertions(+), 6 deletions(-) diff --git a/src/core/include/openvino/pass/pattern/matcher.hpp b/src/core/include/openvino/pass/pattern/matcher.hpp index 659c18be027b1a..7112ac9ff85e64 100644 --- a/src/core/include/openvino/pass/pattern/matcher.hpp +++ b/src/core/include/openvino/pass/pattern/matcher.hpp @@ -63,12 +63,30 @@ class OPENVINO_API Matcher { Matcher(const std::shared_ptr pattern_node, std::nullptr_t name) = delete; Matcher() - : m_match_root{}, m_pattern_node{}, m_pattern_map{}, m_pattern_value_maps{}, m_matched_list{}, m_name{""}, m_strict_mode{false} {} + : m_match_root{}, + m_pattern_node{}, + m_pattern_map{}, + m_pattern_value_maps{}, + m_matched_list{}, + m_name{""}, + m_strict_mode{false} {} Matcher(Output& pattern_node) - : m_match_root{}, m_pattern_node{pattern_node}, m_pattern_map{}, m_pattern_value_maps{}, m_matched_list{}, m_name{""}, m_strict_mode{false} {} + : m_match_root{}, + m_pattern_node{pattern_node}, + m_pattern_map{}, + m_pattern_value_maps{}, + m_matched_list{}, + m_name{""}, + m_strict_mode{false} {} Matcher(Output& pattern_node, const std::string& name) - : m_match_root{}, m_pattern_node{pattern_node}, m_pattern_map{}, m_pattern_value_maps{}, m_matched_list{}, m_name{name}, m_strict_mode{false} {} + : m_match_root{}, + m_pattern_node{pattern_node}, + m_pattern_map{}, + m_pattern_value_maps{}, + m_matched_list{}, + m_name{name}, + m_strict_mode{false} {} /// \brief Constructs a Matcher object /// @@ -76,7 +94,13 @@ class OPENVINO_API Matcher { /// \param name is a string which is used for logging and disabling a matcher /// \param strict_mode forces a matcher to consider shapes and ET of nodes Matcher(const Output& pattern_node, const std::string& name, bool strict_mode) - : m_match_root{}, m_pattern_node{pattern_node}, m_pattern_map{}, m_pattern_value_maps{}, m_matched_list{}, m_name{name}, m_strict_mode{strict_mode} {} + : m_match_root{}, + m_pattern_node{pattern_node}, + m_pattern_map{}, + m_pattern_value_maps{}, + m_matched_list{}, + m_name{name}, + m_strict_mode{strict_mode} {} // Some matches should start on a node rather than an output. These three constructors // are transition until we work out the right way to do that. diff --git a/src/plugins/intel_gpu/src/kernel_selector/kernels/fully_connected/fully_connected_kernel_bf_tiled.cpp b/src/plugins/intel_gpu/src/kernel_selector/kernels/fully_connected/fully_connected_kernel_bf_tiled.cpp index 890f2ee7fb7619..97700ece73c04b 100644 --- a/src/plugins/intel_gpu/src/kernel_selector/kernels/fully_connected/fully_connected_kernel_bf_tiled.cpp +++ b/src/plugins/intel_gpu/src/kernel_selector/kernels/fully_connected/fully_connected_kernel_bf_tiled.cpp @@ -757,8 +757,6 @@ void FullyConnected_bf_tiled::GetUpdateDispatchDataFunc(KernelData& kd) const { const auto& prim_params = static_cast(params); size_t quantize_grp_size = get_dynamic_quantize_group_size(prim_params); - OPENVINO_ASSERT(quantize_grp_size != 0, "Error: quantize_grp_size is zero."); - size_t output_batch = get_output_aligned_bf_size(prim_params, false).first; // Get index of the added shape-agnostic kernel @@ -807,9 +805,11 @@ void FullyConnected_bf_tiled::GetUpdateDispatchDataFunc(KernelData& kd) const { // quantized input is char type kd.internalBufferSizes.push_back(input_size); // half type of de_quan_scale and activation sum for each quantized group + OPENVINO_ASSERT(quantize_grp_size != 0, "Error: quantize_grp_size is zero."); kd.internalBufferSizes.push_back((input_size / quantize_grp_size) * 2 * 2); } + OPENVINO_ASSERT(quantize_grp_size != 0, "Error: quantize_grp_size is zero."); kd.kernels[0].params.workGroups.global = {std::max((input_size / quantize_grp_size), (size_t)1), 1, 1}; kd.kernels[0].params.workGroups.local = {16, 1, 1}; }