From 051d0e9fab55a2530b364ecea1e98c1705e308de Mon Sep 17 00:00:00 2001 From: zhaozhixuan Date: Thu, 15 Jul 2021 20:43:09 +0800 Subject: [PATCH 1/2] Fix bug of single_op. --- ge/single_op/single_op.cc | 4 ++- ge/single_op/task/op_task.cc | 25 ++++++++++++++++--- ge/single_op/task/op_task.h | 5 ++-- ge/single_op/task/tbe_task_builder.cc | 2 +- .../ge/single_op/single_op_task_unittest.cc | 20 +++++++++++++++ 5 files changed, 48 insertions(+), 8 deletions(-) diff --git a/ge/single_op/single_op.cc b/ge/single_op/single_op.cc index a82c30ba..23f4cfad 100755 --- a/ge/single_op/single_op.cc +++ b/ge/single_op/single_op.cc @@ -433,11 +433,13 @@ Status DynamicSingleOp::ExecuteAsync(const vector &input_desc, if (!inputs_size.empty()) { StreamResource *stream_resource = SingleOpManager::GetInstance().GetResource(resource_id_, stream_); GE_CHK_STATUS_RET_NOLOG(UpdateInputsBufferAddr(stream_resource, stream_, inputs_size, update_buffers)); - GE_CHK_STATUS_RET_NOLOG(SetHostTensorValue(input_desc, input_buffers)); } if (hybrid_model_executor_ != nullptr) { GELOGD("Execute multi-task dynamic single op by hybrid model executor"); + if (!inputs_size.empty()) { + GE_CHK_STATUS_RET_NOLOG(SetHostTensorValue(input_desc, input_buffers)); + } hybrid::HybridModelExecutor::ExecuteArgs args; GE_CHK_STATUS_RET_NOLOG(InitHybridModelArgs(update_buffers, output_buffers, input_desc, args)); diff --git a/ge/single_op/task/op_task.cc b/ge/single_op/task/op_task.cc index dbc90ac5..fd6639a5 100755 --- a/ge/single_op/task/op_task.cc +++ b/ge/single_op/task/op_task.cc @@ -294,16 +294,15 @@ Status TbeOpTask::UpdateNodeByShape(const vector &input_desc, cons Status TbeOpTask::EnableDynamicSupport(const NodePtr &node, void *tiling_buffer, uint32_t max_tiling_size) { if (tiling_buffer != nullptr) { - uintptr_t *arg_base = nullptr; - size_t arg_num = 0; - GetIoAddr(arg_base, arg_num); + uintptr_t *arg_base = reinterpret_cast(args_.get()); + size_t arg_num = arg_size_ / sizeof(void *); GE_CHECK_NOTNULL(node); GE_CHECK_NOTNULL(node->GetOpDesc()); uint32_t inputs_num = node->GetOpDesc()->GetInputsSize(); uint32_t outputs_num = node->GetOpDesc()->GetOutputsSize(); uint32_t workspace_nums = node->GetOpDesc()->GetWorkspace().size(); uint32_t tiling_index = inputs_num + outputs_num + workspace_nums; - if (arg_num == 0 || arg_num < tiling_index) { + if (arg_num == 0 || arg_num <= tiling_index) { GELOGE(ACL_ERROR_GE_INTERNAL_ERROR, "[Check][Size]Tiling index %u, arg number %zu is invalid.", tiling_index, arg_num); return ACL_ERROR_GE_INTERNAL_ERROR; @@ -481,6 +480,24 @@ void TbeOpTask::GetIoAddr(uintptr_t *&arg_base, size_t &arg_count) { } } +Status AtomicAddrCleanOpTask::EnableDynamicSupport(const NodePtr &node, void *tiling_buffer, uint32_t max_tiling_size) { + if (tiling_buffer != nullptr) { + uintptr_t *arg_base = reinterpret_cast(args_.get()); + size_t arg_num = arg_size_ / sizeof(void *); + uint32_t tiling_index = atomic_output_indices_.size(); + if (arg_num == 0 || arg_num <= tiling_index) { + GELOGE(ACL_ERROR_GE_INTERNAL_ERROR, "[Check][Size]Tiling index %u, arg number %zu is invalid.", + tiling_index, arg_num); + return ACL_ERROR_GE_INTERNAL_ERROR; + } + arg_base[tiling_index] = reinterpret_cast(tiling_buffer); + } + node_ = node; + tiling_buffer_ = tiling_buffer; + max_tiling_size_ = max_tiling_size; + return SUCCESS; +} + Status AtomicAddrCleanOpTask::UpdateNodeByShape(const vector &input_desc, const vector &output_desc) { return SUCCESS; diff --git a/ge/single_op/task/op_task.h b/ge/single_op/task/op_task.h index 132672b0..4a839389 100644 --- a/ge/single_op/task/op_task.h +++ b/ge/single_op/task/op_task.h @@ -97,7 +97,7 @@ class TbeOpTask : public OpTask { const void *GetArgs() const; size_t GetArgSize() const; const std::string &GetStubName() const; - Status EnableDynamicSupport(const NodePtr &node, void *tiling_buffer, uint32_t max_tiling_size); + virtual Status EnableDynamicSupport(const NodePtr &node, void *tiling_buffer, uint32_t max_tiling_size); const std::string &GetTaskType() const override; void SetHandle(void *handle); @@ -149,6 +149,7 @@ class TbeOpTask : public OpTask { class AtomicAddrCleanOpTask : public TbeOpTask { public: Status InitAtomicAddrCleanIndices(); + Status EnableDynamicSupport(const NodePtr &node, void *tiling_buffer, uint32_t max_tiling_size) override; private: Status UpdateNodeByShape(const vector &input_desc, @@ -156,8 +157,8 @@ class AtomicAddrCleanOpTask : public TbeOpTask { Status UpdateIoAddr(const vector &inputs, const vector &outputs) override; Status UpdateTilingArgs(rtStream_t stream) override; Status CalcTilingInfo(optiling::utils::OpRunInfo &run_info) override; - std::vector atomic_output_indices_; + std::vector atomic_output_indices_; }; class AiCpuBaseTask : public OpTask { diff --git a/ge/single_op/task/tbe_task_builder.cc b/ge/single_op/task/tbe_task_builder.cc index 017dac25..f947ca57 100644 --- a/ge/single_op/task/tbe_task_builder.cc +++ b/ge/single_op/task/tbe_task_builder.cc @@ -425,7 +425,7 @@ Status TbeTaskBuilder::InitTilingInfo(TbeOpTask &task) { GELOGD("[%s] Done allocating tiling buffer, size=%ld.", op_desc_->GetName().c_str(), max_size); } - task.EnableDynamicSupport(node_, tiling_buffer, static_cast(max_size)); + GE_CHK_STATUS_RET_NOLOG(task.EnableDynamicSupport(node_, tiling_buffer, static_cast(max_size))); return SUCCESS; } diff --git a/tests/ut/ge/single_op/single_op_task_unittest.cc b/tests/ut/ge/single_op/single_op_task_unittest.cc index 8964df74..5960fbbc 100644 --- a/tests/ut/ge/single_op/single_op_task_unittest.cc +++ b/tests/ut/ge/single_op/single_op_task_unittest.cc @@ -237,3 +237,23 @@ TEST_F(UtestSingleOpTask, test_aicpu_task_update_io_addr) { ASSERT_EQ(ret, PARAM_INVALID); } } + +TEST_F(UtestSingleOpTask, test_dynamic_support) { + auto graph = make_shared("graph"); + auto op_desc = make_shared("Add", "Add"); + auto node = graph->AddNode(op_desc); + AtomicAddrCleanOpTask atomic_task; + TbeOpTask tbe_task; + + ASSERT_EQ(tbe_task.EnableDynamicSupport(node, (void *)0x0001, 1), ACL_ERROR_GE_INTERNAL_ERROR); + ASSERT_EQ(atomic_task.EnableDynamicSupport(node, (void *)0x0001, 1), ACL_ERROR_GE_INTERNAL_ERROR); + + tbe_task.arg_size_ = sizeof(void *); + tbe_task.args_.reset(new (std::nothrow) uint8_t[tbe_task.arg_size_]); + atomic_task.arg_size_ = sizeof(void *); + atomic_task.args_.reset(new (std::nothrow) uint8_t[atomic_task.arg_size_]); + ASSERT_EQ(tbe_task.EnableDynamicSupport(node, (void *)0x0001, 1), SUCCESS); + ASSERT_EQ(atomic_task.EnableDynamicSupport(node, (void *)0x0001, 1), SUCCESS); + tbe_task.tiling_buffer_ = nullptr; + atomic_task.tiling_buffer_ = nullptr; +} From 21886e608e12983bbe3aecf74e053ed1707ce121 Mon Sep 17 00:00:00 2001 From: zhaozhixuan Date: Fri, 16 Jul 2021 12:00:31 +0800 Subject: [PATCH 2/2] Fix review advice. --- ge/single_op/task/op_task.cc | 26 ++++++++++--------- .../ge/single_op/single_op_task_unittest.cc | 8 ++++-- 2 files changed, 20 insertions(+), 14 deletions(-) diff --git a/ge/single_op/task/op_task.cc b/ge/single_op/task/op_task.cc index fd6639a5..ee752022 100755 --- a/ge/single_op/task/op_task.cc +++ b/ge/single_op/task/op_task.cc @@ -293,25 +293,26 @@ Status TbeOpTask::UpdateNodeByShape(const vector &input_desc, cons } Status TbeOpTask::EnableDynamicSupport(const NodePtr &node, void *tiling_buffer, uint32_t max_tiling_size) { + node_ = node; + tiling_buffer_ = tiling_buffer; + max_tiling_size_ = max_tiling_size; if (tiling_buffer != nullptr) { - uintptr_t *arg_base = reinterpret_cast(args_.get()); - size_t arg_num = arg_size_ / sizeof(void *); + uintptr_t *arg_base = nullptr; + size_t arg_num = 0; + GetIoAddr(arg_base, arg_num); GE_CHECK_NOTNULL(node); GE_CHECK_NOTNULL(node->GetOpDesc()); uint32_t inputs_num = node->GetOpDesc()->GetInputsSize(); uint32_t outputs_num = node->GetOpDesc()->GetOutputsSize(); uint32_t workspace_nums = node->GetOpDesc()->GetWorkspace().size(); uint32_t tiling_index = inputs_num + outputs_num + workspace_nums; - if (arg_num == 0 || arg_num <= tiling_index) { + if (arg_num == 0 || arg_num < tiling_index) { GELOGE(ACL_ERROR_GE_INTERNAL_ERROR, "[Check][Size]Tiling index %u, arg number %zu is invalid.", tiling_index, arg_num); return ACL_ERROR_GE_INTERNAL_ERROR; } arg_base[tiling_index] = reinterpret_cast(tiling_buffer); } - node_ = node; - tiling_buffer_ = tiling_buffer; - max_tiling_size_ = max_tiling_size; return SUCCESS; } @@ -481,20 +482,21 @@ void TbeOpTask::GetIoAddr(uintptr_t *&arg_base, size_t &arg_count) { } Status AtomicAddrCleanOpTask::EnableDynamicSupport(const NodePtr &node, void *tiling_buffer, uint32_t max_tiling_size) { + node_ = node; + tiling_buffer_ = tiling_buffer; + max_tiling_size_ = max_tiling_size; if (tiling_buffer != nullptr) { - uintptr_t *arg_base = reinterpret_cast(args_.get()); - size_t arg_num = arg_size_ / sizeof(void *); + uintptr_t *arg_base = nullptr; + size_t arg_num = 0; + GetIoAddr(arg_base, arg_num); uint32_t tiling_index = atomic_output_indices_.size(); - if (arg_num == 0 || arg_num <= tiling_index) { + if (arg_num == 0 || arg_num < tiling_index) { GELOGE(ACL_ERROR_GE_INTERNAL_ERROR, "[Check][Size]Tiling index %u, arg number %zu is invalid.", tiling_index, arg_num); return ACL_ERROR_GE_INTERNAL_ERROR; } arg_base[tiling_index] = reinterpret_cast(tiling_buffer); } - node_ = node; - tiling_buffer_ = tiling_buffer; - max_tiling_size_ = max_tiling_size; return SUCCESS; } diff --git a/tests/ut/ge/single_op/single_op_task_unittest.cc b/tests/ut/ge/single_op/single_op_task_unittest.cc index 5960fbbc..9a0381cd 100644 --- a/tests/ut/ge/single_op/single_op_task_unittest.cc +++ b/tests/ut/ge/single_op/single_op_task_unittest.cc @@ -245,12 +245,16 @@ TEST_F(UtestSingleOpTask, test_dynamic_support) { AtomicAddrCleanOpTask atomic_task; TbeOpTask tbe_task; + tbe_task.arg_size_ = sizeof(void *) * 1; + tbe_task.args_.reset(new (std::nothrow) uint8_t[tbe_task.arg_size_]); + atomic_task.arg_size_ = sizeof(void *) * 1; + atomic_task.args_.reset(new (std::nothrow) uint8_t[atomic_task.arg_size_]); ASSERT_EQ(tbe_task.EnableDynamicSupport(node, (void *)0x0001, 1), ACL_ERROR_GE_INTERNAL_ERROR); ASSERT_EQ(atomic_task.EnableDynamicSupport(node, (void *)0x0001, 1), ACL_ERROR_GE_INTERNAL_ERROR); - tbe_task.arg_size_ = sizeof(void *); + tbe_task.arg_size_ = sizeof(void *) * 2; tbe_task.args_.reset(new (std::nothrow) uint8_t[tbe_task.arg_size_]); - atomic_task.arg_size_ = sizeof(void *); + atomic_task.arg_size_ = sizeof(void *) * 2; atomic_task.args_.reset(new (std::nothrow) uint8_t[atomic_task.arg_size_]); ASSERT_EQ(tbe_task.EnableDynamicSupport(node, (void *)0x0001, 1), SUCCESS); ASSERT_EQ(atomic_task.EnableDynamicSupport(node, (void *)0x0001, 1), SUCCESS);