From 5687e55250613417c151864cb74229fc91ea6462 Mon Sep 17 00:00:00 2001 From: SiCong Li Date: Wed, 17 Aug 2022 17:09:05 +0100 Subject: Fix invalid memory access for dynamically fused Cl Elementwise kernels The M0 and N0 were incorrectly set for the case of broadcasting when the elementwise component is non-root. This is because we previously always use rhs tensor to derive the load M0, N0. But for non-root components, the addend/divisor tensor can be in the lhs or rhs. Thus this would fail in case the addend/divisor is in the lhs. - Also fixes broken Dynamic Fusion test Resolves COMPMID-5482 Signed-off-by: SiCong Li Change-Id: I37f27ffa392781387db15739b1666f1dad28c554 Reviewed-on: https://eu-gerrit-1.euhpc.arm.com/c/VisualCompute/ComputeLibrary/+/445890 Tested-by: bsgcomp Reviewed-by: Mohammed Suhail Munshi Comments-Addressed: bsgcomp Reviewed-on: https://review.mlplatform.org/c/ml/ComputeLibrary/+/8111 Tested-by: Arm Jenkins Reviewed-by: Pablo Marquez Tello Reviewed-by: Gunes Bayir Comments-Addressed: Arm Jenkins Benchmark: Arm Jenkins --- .../components/ClElementwiseKernelComponent.cpp | 20 +++++++++++--------- .../CL/UNIT/dynamic_fusion/ClCompositeKernel.cpp | 2 +- 2 files changed, 12 insertions(+), 10 deletions(-) diff --git a/src/core/experimental/dynamic_fusion/ClKernelBuildingImpl/components/ClElementwiseKernelComponent.cpp b/src/core/experimental/dynamic_fusion/ClKernelBuildingImpl/components/ClElementwiseKernelComponent.cpp index 7515aec27a..e2eba68a63 100644 --- a/src/core/experimental/dynamic_fusion/ClKernelBuildingImpl/components/ClElementwiseKernelComponent.cpp +++ b/src/core/experimental/dynamic_fusion/ClKernelBuildingImpl/components/ClElementwiseKernelComponent.cpp @@ -175,16 +175,17 @@ void ClElementwiseKernelComponent::allocate_shared_vars(SharedVarTable &vtable) ClElementwiseKernelComponent::TagLUT ClElementwiseKernelComponent::get_tag_lut(const SharedVarTable &vtable) const { - TagLUT lut{}; - const auto t_dst_info = _blueprint->impl().get_kernel_argument_info(_blueprint->impl().get_dst_id()); - const auto t_rhs_info = _blueprint->impl().get_kernel_argument_info(_rhs.arg_id); + TagLUT lut{}; + const auto t_dst_info = _blueprint->impl().get_kernel_argument_info(_blueprint->impl().get_dst_id()); + ITensorInfo *t_addend_info = nullptr; // Arguments and global shared variables const bool is_root = _blueprint->impl().group(_lhs.arg_id) == SharedVarGroup::Argument && _blueprint->impl().group(_rhs.arg_id) == SharedVarGroup::Argument; if(is_root) { - lut["lhs"] = vtable.get(_lhs); - lut["rhs"] = vtable.get(_rhs); - lut["dst"] = vtable.get(_dst); + lut["lhs"] = vtable.get(_lhs); + lut["rhs"] = vtable.get(_rhs); + lut["dst"] = vtable.get(_dst); + t_addend_info = _blueprint->impl().get_kernel_argument_info(_rhs.arg_id); } else { @@ -207,6 +208,7 @@ ClElementwiseKernelComponent::TagLUT ClElementwiseKernelComponent::get_tag_lut(c } lut["acc"] = vtable.get(accumulator); lut["addend"] = vtable.get(addend); + t_addend_info = _blueprint->impl().get_kernel_argument_info(addend.arg_id); } // Local build options lut["meta_kernel_id"] = id(); @@ -226,18 +228,18 @@ ClElementwiseKernelComponent::TagLUT ClElementwiseKernelComponent::get_tag_lut(c // Set broadcast parameters // PRE: All tensors are broadcast-compatible - const bool is_broadcast = t_rhs_info->tensor_shape() != t_dst_info->tensor_shape(); + const bool is_broadcast = t_addend_info->tensor_shape() != t_dst_info->tensor_shape(); if(is_broadcast) { // Note that n0 maps to input tensor dimension 0, m0 maps to input dimensions 1 and 2 because of our collapse strategy - if(t_rhs_info->dimension(0) == 1U && t_rhs_info->dimension(1) == 1U && t_rhs_info->dimension(2) == 1U) // Broadcast in X, Y, Z: collapsed rhs win [M0xN0] = [1x1] + if(t_addend_info->dimension(0) == 1U && t_addend_info->dimension(1) == 1U && t_addend_info->dimension(2) == 1U) // Broadcast in X, Y, Z: collapsed rhs win [M0xN0] = [1x1] { lut["rhs_m0"] = "1"; lut["rhs_n0"] = "1"; lut["rhs_start_y"] = "0"; lut["rhs_start_x"] = "0"; } - else if(t_rhs_info->dimension(1) == 1U && t_rhs_info->dimension(2) == 1U) // Broadcast in Y and Z: collapsed rhs win [M0xN0] = [1xN] + else if(t_addend_info->dimension(1) == 1U && t_addend_info->dimension(2) == 1U) // Broadcast in Y and Z: collapsed rhs win [M0xN0] = [1xN] { lut["rhs_m0"] = "1"; lut["rhs_n0"] = "N0"; diff --git a/tests/validation/CL/UNIT/dynamic_fusion/ClCompositeKernel.cpp b/tests/validation/CL/UNIT/dynamic_fusion/ClCompositeKernel.cpp index 3ffbc077c6..dc98d72f4b 100644 --- a/tests/validation/CL/UNIT/dynamic_fusion/ClCompositeKernel.cpp +++ b/tests/validation/CL/UNIT/dynamic_fusion/ClCompositeKernel.cpp @@ -171,7 +171,7 @@ TEST_CASE(MoveNet_SubGraph_1_DirectConv2d, framework::DatasetMode::ALL) SimpleTensor ref_src_nhwc{ src_shape, data_type, 1, QuantizationInfo(), DataLayout::NHWC }; SimpleTensor ref_wei_nhwc{ wei_shape, data_type, 1, QuantizationInfo(), DataLayout::NHWC }; SimpleTensor ref_bia_nhwc{ bia_shape, data_type, 1, QuantizationInfo(), DataLayout::NHWC }; - SimpleTensor ref_addend_nhwc{ dst_shape, data_type, 1, QuantizationInfo(), DataLayout::NHWC }; + SimpleTensor ref_addend_nhwc{ addend_shape, data_type, 1, QuantizationInfo(), DataLayout::NHWC }; // Fill reference fill(ref_src_nhwc, 0, library.get()); -- cgit v1.2.1