From d11099c2a9857108f195163db5c57786e207cd39 Mon Sep 17 00:00:00 2001 From: Juntian Liu Date: Thu, 22 May 2025 13:17:23 -0700 Subject: [PATCH] Refactor write_tensor_or_raise_error to Return Error Codes Instead of Raising Errors (#11051) Summary: Pull Request resolved: https://github.com/pytorch/executorch/pull/11051 This PR to make the write_tensor_or_raise_error function error out more gracefully by updating the signature of write_tensor_or_raise_error to return executorch::runtime::Result instead of long Check the error code and raise an error if needed after invoking write_tensor_or_return_error function using ET_CHECK_MSG Reviewed By: Gasoonjia Differential Revision: D75111482 --- devtools/etdump/etdump_flatcc.cpp | 42 +++++++++++++++++---------- devtools/etdump/etdump_flatcc.h | 2 +- devtools/etdump/tests/etdump_test.cpp | 6 ++-- 3 files changed, 31 insertions(+), 19 deletions(-) diff --git a/devtools/etdump/etdump_flatcc.cpp b/devtools/etdump/etdump_flatcc.cpp index 4b5da78550e..ccca2beb257 100644 --- a/devtools/etdump/etdump_flatcc.cpp +++ b/devtools/etdump/etdump_flatcc.cpp @@ -394,9 +394,12 @@ Result ETDumpGen::log_intermediate_output_delegate_helper( // Check the type of `output` then call the corresponding logging functions if constexpr (std::is_same::value) { - long offset = write_tensor_or_raise_error(output); + Result offset = write_tensor_or_return_error(output); + ET_CHECK_MSG( + offset.ok(), + "write_tensor_or_return_error() failed to write tensor to debug buffer"); Result tensor_ref = - add_tensor_entry(builder_, output, offset); + add_tensor_entry(builder_, output, offset.get()); if (!tensor_ref.ok()) { return tensor_ref.error(); } @@ -408,9 +411,12 @@ Result ETDumpGen::log_intermediate_output_delegate_helper( } else if constexpr (std::is_same>::value) { etdump_Tensor_vec_start(builder_); for (size_t i = 0; i < output.size(); ++i) { - long offset = write_tensor_or_raise_error(output[i]); + Result offset = write_tensor_or_return_error(output[i]); + ET_CHECK_MSG( + offset.ok(), + "write_tensor_or_return_error() failed to write tensor to debug buffer"); Result tensor_ref = - add_tensor_entry(builder_, output[i], offset); + add_tensor_entry(builder_, output[i], offset.get()); if (!tensor_ref.ok()) { return tensor_ref.error(); } @@ -566,9 +572,12 @@ Result ETDumpGen::log_evalue( switch (evalue.tag) { case Tag::Tensor: { executorch::aten::Tensor tensor = evalue.toTensor(); - long offset = write_tensor_or_raise_error(tensor); + Result offset = write_tensor_or_return_error(tensor); + ET_CHECK_MSG( + offset.ok(), + "write_tensor_or_return_error() failed to write tensor to debug buffer"); Result tensor_ref = - add_tensor_entry(builder_, tensor, offset); + add_tensor_entry(builder_, tensor, offset.get()); if (!tensor_ref.ok()) { return tensor_ref.error(); } @@ -591,9 +600,12 @@ Result ETDumpGen::log_evalue( evalue.toTensorList(); etdump_Tensor_vec_start(builder_); for (size_t i = 0; i < tensors.size(); ++i) { - long offset = write_tensor_or_raise_error(tensors[i]); + Result offset = write_tensor_or_return_error(tensors[i]); + ET_CHECK_MSG( + offset.ok(), + "write_tensor_or_return_error() failed to write tensor to debug buffer"); Result tensor_ref = - add_tensor_entry(builder_, tensors[i], offset); + add_tensor_entry(builder_, tensors[i], offset.get()); if (!tensor_ref.ok()) { return tensor_ref.error(); } @@ -689,7 +701,7 @@ void ETDumpGen::set_delegation_intermediate_output_filter( filter_ = filter; } -long ETDumpGen::write_tensor_or_raise_error(Tensor tensor) { +Result ETDumpGen::write_tensor_or_return_error(Tensor tensor) { // Previously, the function copy_tensor_to_debug_buffer returned 0xFF..F when // given an empty tensor, which is an invalid offset for most buffers. In our // data sink, we will return the current debug_buffer_offset for better @@ -702,14 +714,14 @@ long ETDumpGen::write_tensor_or_raise_error(Tensor tensor) { return static_cast(-1); } - ET_CHECK_MSG( - data_sink_, "Must set data sink before writing tensor-like data"); + if (!data_sink_) { + return Error::InvalidArgument; + } Result ret = data_sink_->write(tensor.const_data_ptr(), tensor.nbytes()); - ET_CHECK_MSG( - ret.ok(), - "Failed to write tensor with error 0x%" PRIx32, - static_cast(ret.error())); + if (!ret.ok()) { + return ret.error(); + } return static_cast(ret.get()); } diff --git a/devtools/etdump/etdump_flatcc.h b/devtools/etdump/etdump_flatcc.h index ea0c1cb653d..8b39b243165 100644 --- a/devtools/etdump/etdump_flatcc.h +++ b/devtools/etdump/etdump_flatcc.h @@ -184,7 +184,7 @@ class ETDumpGen : public ::executorch::runtime::EventTracer { DelegateDebugIntId delegate_debug_index, const T& output); - long write_tensor_or_raise_error(executorch::aten::Tensor tensor); + Result write_tensor_or_return_error(executorch::aten::Tensor tensor); struct flatcc_builder* builder_; size_t num_blocks_ = 0; diff --git a/devtools/etdump/tests/etdump_test.cpp b/devtools/etdump/tests/etdump_test.cpp index 9d39a8bbde1..d095844986f 100644 --- a/devtools/etdump/tests/etdump_test.cpp +++ b/devtools/etdump/tests/etdump_test.cpp @@ -76,7 +76,7 @@ class ProfilerETDumpTest : public ::testing::Test { ET_EXPECT_DEATH( gen->log_intermediate_output_delegate( "test_event_tensor", kUnsetDelegateDebugIntId, tf.ones({3, 2})), - "Must set data sink before writing tensor-like data"); + "failed to write tensor to debug buffer"); } void check_log_with_filter( @@ -301,7 +301,7 @@ TEST_F(ProfilerETDumpTest, DebugEvent) { if (j == 0) { ET_EXPECT_DEATH( etdump_gen[i]->log_evalue(evalue_tensor), - "Must set data sink before writing tensor-like data"); + "failed to write tensor to debug buffer"); // Set debug buffer with span etdump_gen[i]->set_debug_buffer(buffer); @@ -315,7 +315,7 @@ TEST_F(ProfilerETDumpTest, DebugEvent) { ET_EXPECT_DEATH( etdump_gen[i]->log_evalue(evalue_tensor), - "Must set data sink before writing tensor-like data"); + "failed to write tensor to debug buffer"); if (j == 1) { // Set buffer data sink