From 6e72390098abaecc90d6d54a117f7daac4e243de Mon Sep 17 00:00:00 2001 From: zhenyanzhang Date: Wed, 9 Apr 2025 23:00:51 -0700 Subject: [PATCH 1/2] [#9971] Gracefully error out in ETDump for get_flatbuffer_scalar_type Following https://github.com/pytorch/executorch/issues/9971 - Update get_flatbuffer_scalar_type return type to Result - Iteratively update functions that calling the functions with result type changed: - Check returns, if with an error, pass above the error. - If unable to pass error, update the return type as Result Differential Revision: [D72771753](https://our.internmc.facebook.com/intern/diff/D72771753/) [ghstack-poisoned] --- devtools/etdump/etdump_flatcc.cpp | 51 ++++++++++++++++++------- devtools/etdump/etdump_flatcc.h | 2 +- runtime/core/event_tracer.h | 2 +- runtime/core/test/event_tracer_test.cpp | 4 +- 4 files changed, 42 insertions(+), 17 deletions(-) diff --git a/devtools/etdump/etdump_flatcc.cpp b/devtools/etdump/etdump_flatcc.cpp index f6e18bf49d2..62c826fe215 100644 --- a/devtools/etdump/etdump_flatcc.cpp +++ b/devtools/etdump/etdump_flatcc.cpp @@ -42,7 +42,7 @@ namespace executorch { namespace etdump { namespace { -executorch_flatbuffer_ScalarType_enum_t get_flatbuffer_scalar_type( +Result get_flatbuffer_scalar_type( executorch::aten::ScalarType tensor_scalar_type) { switch (tensor_scalar_type) { case executorch::aten::ScalarType::Byte: @@ -66,21 +66,26 @@ executorch_flatbuffer_ScalarType_enum_t get_flatbuffer_scalar_type( case executorch::aten::ScalarType::UInt16: return executorch_flatbuffer_ScalarType_UINT16; default: - ET_CHECK_MSG( + ET_CHECK_OR_RETURN_ERROR( 0, + InvalidArgument, "This ScalarType = %hhd is not yet supported in ETDump", static_cast(tensor_scalar_type)); } } -etdump_Tensor_ref_t add_tensor_entry( +Result add_tensor_entry( flatcc_builder_t* builder_, const executorch::aten::Tensor& tensor, long offset) { etdump_Tensor_start(builder_); - etdump_Tensor_scalar_type_add( - builder_, get_flatbuffer_scalar_type(tensor.scalar_type())); + Result scalar_type = + get_flatbuffer_scalar_type(tensor.scalar_type()); + if (!scalar_type.ok()) { + return scalar_type.error(); + } + etdump_Tensor_scalar_type_add(builder_, scalar_type.get()); etdump_Tensor_sizes_start(builder_); for (auto dim : tensor.sizes()) { @@ -398,18 +403,26 @@ 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); - etdump_Tensor_ref_t tensor_ref = add_tensor_entry(builder_, output, offset); + Result tensor_ref = + add_tensor_entry(builder_, output, offset); + if (!tensor_ref.ok()) { + return tensor_ref.error(); + } etdump_Value_start(builder_); etdump_Value_val_add(builder_, etdump_ValueType_Tensor); - etdump_Value_tensor_add(builder_, tensor_ref); + etdump_Value_tensor_add(builder_, tensor_ref.get()); } 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]); - etdump_Tensor_vec_push( - builder_, add_tensor_entry(builder_, output[i], offset)); + Result tensor_ref = + add_tensor_entry(builder_, output[i], offset); + if (!tensor_ref.ok()) { + return tensor_ref.error(); + } + etdump_Tensor_vec_push(builder_, tensor_ref.get()); } etdump_Tensor_vec_ref_t tensor_vec_ref = etdump_Tensor_vec_end(builder_); etdump_TensorList_ref_t tensor_list_ref = @@ -546,7 +559,9 @@ void ETDumpGen::set_data_sink(DataSinkBase* data_sink) { data_sink_ = data_sink; } -void ETDumpGen::log_evalue(const EValue& evalue, LoggedEValueType evalue_type) { +Result ETDumpGen::log_evalue( + const EValue& evalue, + LoggedEValueType evalue_type) { check_ready_to_add_events(); etdump_DebugEvent_start(builder_); @@ -558,12 +573,15 @@ void ETDumpGen::log_evalue(const EValue& evalue, LoggedEValueType evalue_type) { case Tag::Tensor: { executorch::aten::Tensor tensor = evalue.toTensor(); long offset = write_tensor_or_raise_error(tensor); - etdump_Tensor_ref_t tensor_ref = + Result tensor_ref = add_tensor_entry(builder_, tensor, offset); + if (!tensor_ref.ok()) { + return tensor_ref.error(); + } etdump_Value_start(builder_); etdump_Value_val_add(builder_, etdump_ValueType_Tensor); - etdump_Value_tensor_add(builder_, tensor_ref); + etdump_Value_tensor_add(builder_, tensor_ref.get()); if (evalue_type == LoggedEValueType::kProgramOutput) { auto bool_ref = etdump_Bool_create(builder_, FLATBUFFERS_TRUE); etdump_Value_output_add(builder_, bool_ref); @@ -580,8 +598,12 @@ void ETDumpGen::log_evalue(const EValue& evalue, LoggedEValueType evalue_type) { etdump_Tensor_vec_start(builder_); for (size_t i = 0; i < tensors.size(); ++i) { long offset = write_tensor_or_raise_error(tensors[i]); - etdump_Tensor_vec_push( - builder_, add_tensor_entry(builder_, tensors[i], offset)); + Result tensor_ref = + add_tensor_entry(builder_, tensors[i], offset); + if (!tensor_ref.ok()) { + return tensor_ref.error(); + } + etdump_Tensor_vec_push(builder_, tensor_ref.get()); } etdump_Tensor_vec_ref_t tensor_vec_ref = etdump_Tensor_vec_end(builder_); etdump_TensorList_ref_t tensor_list_ref = @@ -653,6 +675,7 @@ void ETDumpGen::log_evalue(const EValue& evalue, LoggedEValueType evalue_type) { etdump_RunData_events_push_start(builder_); etdump_Event_debug_event_add(builder_, debug_event); etdump_RunData_events_push_end(builder_); + return true; } size_t ETDumpGen::get_num_blocks() { diff --git a/devtools/etdump/etdump_flatcc.h b/devtools/etdump/etdump_flatcc.h index dc0f822f922..3a186b5bbc3 100644 --- a/devtools/etdump/etdump_flatcc.h +++ b/devtools/etdump/etdump_flatcc.h @@ -102,7 +102,7 @@ class ETDumpGen : public ::executorch::runtime::EventTracer { size_t size) override; virtual ::executorch::runtime::AllocatorID track_allocator( const char* name) override; - virtual void log_evalue( + virtual Result log_evalue( const ::executorch::runtime::EValue& evalue, ::executorch::runtime::LoggedEValueType evalue_type = ::executorch::runtime::LoggedEValueType::kIntermediateOutput) diff --git a/runtime/core/event_tracer.h b/runtime/core/event_tracer.h index e810437f7d2..0365606d0c4 100644 --- a/runtime/core/event_tracer.h +++ b/runtime/core/event_tracer.h @@ -314,7 +314,7 @@ class EventTracer { * @param[in] evalue_type Indicates what type of output this is logging e.g. * an intermediate output, program output etc. */ - virtual void log_evalue( + virtual Result log_evalue( const EValue& evalue, LoggedEValueType evalue_type) = 0; diff --git a/runtime/core/test/event_tracer_test.cpp b/runtime/core/test/event_tracer_test.cpp index ee16aa924a6..1c9e1a446b9 100644 --- a/runtime/core/test/event_tracer_test.cpp +++ b/runtime/core/test/event_tracer_test.cpp @@ -161,9 +161,11 @@ class DummyEventTracer : public EventTracer { return true; } - void log_evalue(const EValue& evalue, LoggedEValueType evalue_type) override { + Result log_evalue(const EValue& evalue, LoggedEValueType evalue_type) + override { logged_evalue_ = evalue; logged_evalue_type_ = evalue_type; + return true; } EValue logged_evalue() { From d52cefdca7037b89862d8504b5b49f62120afdb5 Mon Sep 17 00:00:00 2001 From: zhenyanzhang Date: Thu, 10 Apr 2025 00:23:59 -0700 Subject: [PATCH 2/2] Update on "[#9971] Gracefully error out in ETDump for get_flatbuffer_scalar_type" Following https://github.com/pytorch/executorch/issues/9971 - Update get_flatbuffer_scalar_type return type to Result - Iteratively update functions that calling the functions with result type changed: - Check returns, if with an error, pass above the error. - If unable to pass error, update the return type as Result Differential Revision: [D72771753](https://our.internmc.facebook.com/intern/diff/D72771753/) [ghstack-poisoned] --- runtime/core/event_tracer.h | 3 +++ 1 file changed, 3 insertions(+) diff --git a/runtime/core/event_tracer.h b/runtime/core/event_tracer.h index 0365606d0c4..5bcdd0cfb1f 100644 --- a/runtime/core/event_tracer.h +++ b/runtime/core/event_tracer.h @@ -313,6 +313,9 @@ class EventTracer { * @param[in] evalue The value to be logged. * @param[in] evalue_type Indicates what type of output this is logging e.g. * an intermediate output, program output etc. + * @return A Result indicating the status of the logging operation. + * - True if the evalue output was successfully logged. + * - An error code if an error occurs during logging. */ virtual Result log_evalue( const EValue& evalue,