Skip to content

Commit 2ed44ce

Browse files
authored
[#9971] Gracefully error out in ETDump for get_flatbuffer_scalar_type (#10076)
Following #9971 - Update get_flatbuffer_scalar_type return type to Result<T> - 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<T> Differential Revision: [D72771753](https://our.internmc.facebook.com/intern/diff/D72771753/)
1 parent 060cda3 commit 2ed44ce

File tree

4 files changed

+45
-17
lines changed

4 files changed

+45
-17
lines changed

devtools/etdump/etdump_flatcc.cpp

Lines changed: 37 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ namespace executorch {
4242
namespace etdump {
4343
namespace {
4444

45-
executorch_flatbuffer_ScalarType_enum_t get_flatbuffer_scalar_type(
45+
Result<executorch_flatbuffer_ScalarType_enum_t> get_flatbuffer_scalar_type(
4646
executorch::aten::ScalarType tensor_scalar_type) {
4747
switch (tensor_scalar_type) {
4848
case executorch::aten::ScalarType::Byte:
@@ -66,21 +66,26 @@ executorch_flatbuffer_ScalarType_enum_t get_flatbuffer_scalar_type(
6666
case executorch::aten::ScalarType::UInt16:
6767
return executorch_flatbuffer_ScalarType_UINT16;
6868
default:
69-
ET_CHECK_MSG(
69+
ET_CHECK_OR_RETURN_ERROR(
7070
0,
71+
InvalidArgument,
7172
"This ScalarType = %hhd is not yet supported in ETDump",
7273
static_cast<char>(tensor_scalar_type));
7374
}
7475
}
7576

76-
etdump_Tensor_ref_t add_tensor_entry(
77+
Result<etdump_Tensor_ref_t> add_tensor_entry(
7778
flatcc_builder_t* builder_,
7879
const executorch::aten::Tensor& tensor,
7980
long offset) {
8081
etdump_Tensor_start(builder_);
8182

82-
etdump_Tensor_scalar_type_add(
83-
builder_, get_flatbuffer_scalar_type(tensor.scalar_type()));
83+
Result<executorch_flatbuffer_ScalarType_enum_t> scalar_type =
84+
get_flatbuffer_scalar_type(tensor.scalar_type());
85+
if (!scalar_type.ok()) {
86+
return scalar_type.error();
87+
}
88+
etdump_Tensor_scalar_type_add(builder_, scalar_type.get());
8489
etdump_Tensor_sizes_start(builder_);
8590

8691
for (auto dim : tensor.sizes()) {
@@ -390,18 +395,26 @@ Result<bool> ETDumpGen::log_intermediate_output_delegate_helper(
390395
// Check the type of `output` then call the corresponding logging functions
391396
if constexpr (std::is_same<T, Tensor>::value) {
392397
long offset = write_tensor_or_raise_error(output);
393-
etdump_Tensor_ref_t tensor_ref = add_tensor_entry(builder_, output, offset);
398+
Result<etdump_Tensor_ref_t> tensor_ref =
399+
add_tensor_entry(builder_, output, offset);
400+
if (!tensor_ref.ok()) {
401+
return tensor_ref.error();
402+
}
394403

395404
etdump_Value_start(builder_);
396405
etdump_Value_val_add(builder_, etdump_ValueType_Tensor);
397-
etdump_Value_tensor_add(builder_, tensor_ref);
406+
etdump_Value_tensor_add(builder_, tensor_ref.get());
398407

399408
} else if constexpr (std::is_same<T, ArrayRef<Tensor>>::value) {
400409
etdump_Tensor_vec_start(builder_);
401410
for (size_t i = 0; i < output.size(); ++i) {
402411
long offset = write_tensor_or_raise_error(output[i]);
403-
etdump_Tensor_vec_push(
404-
builder_, add_tensor_entry(builder_, output[i], offset));
412+
Result<etdump_Tensor_ref_t> tensor_ref =
413+
add_tensor_entry(builder_, output[i], offset);
414+
if (!tensor_ref.ok()) {
415+
return tensor_ref.error();
416+
}
417+
etdump_Tensor_vec_push(builder_, tensor_ref.get());
405418
}
406419
etdump_Tensor_vec_ref_t tensor_vec_ref = etdump_Tensor_vec_end(builder_);
407420
etdump_TensorList_ref_t tensor_list_ref =
@@ -538,7 +551,9 @@ void ETDumpGen::set_data_sink(DataSinkBase* data_sink) {
538551
data_sink_ = data_sink;
539552
}
540553

541-
void ETDumpGen::log_evalue(const EValue& evalue, LoggedEValueType evalue_type) {
554+
Result<bool> ETDumpGen::log_evalue(
555+
const EValue& evalue,
556+
LoggedEValueType evalue_type) {
542557
check_ready_to_add_events();
543558

544559
etdump_DebugEvent_start(builder_);
@@ -550,12 +565,15 @@ void ETDumpGen::log_evalue(const EValue& evalue, LoggedEValueType evalue_type) {
550565
case Tag::Tensor: {
551566
executorch::aten::Tensor tensor = evalue.toTensor();
552567
long offset = write_tensor_or_raise_error(tensor);
553-
etdump_Tensor_ref_t tensor_ref =
568+
Result<etdump_Tensor_ref_t> tensor_ref =
554569
add_tensor_entry(builder_, tensor, offset);
570+
if (!tensor_ref.ok()) {
571+
return tensor_ref.error();
572+
}
555573

556574
etdump_Value_start(builder_);
557575
etdump_Value_val_add(builder_, etdump_ValueType_Tensor);
558-
etdump_Value_tensor_add(builder_, tensor_ref);
576+
etdump_Value_tensor_add(builder_, tensor_ref.get());
559577
if (evalue_type == LoggedEValueType::kProgramOutput) {
560578
auto bool_ref = etdump_Bool_create(builder_, FLATBUFFERS_TRUE);
561579
etdump_Value_output_add(builder_, bool_ref);
@@ -572,8 +590,12 @@ void ETDumpGen::log_evalue(const EValue& evalue, LoggedEValueType evalue_type) {
572590
etdump_Tensor_vec_start(builder_);
573591
for (size_t i = 0; i < tensors.size(); ++i) {
574592
long offset = write_tensor_or_raise_error(tensors[i]);
575-
etdump_Tensor_vec_push(
576-
builder_, add_tensor_entry(builder_, tensors[i], offset));
593+
Result<etdump_Tensor_ref_t> tensor_ref =
594+
add_tensor_entry(builder_, tensors[i], offset);
595+
if (!tensor_ref.ok()) {
596+
return tensor_ref.error();
597+
}
598+
etdump_Tensor_vec_push(builder_, tensor_ref.get());
577599
}
578600
etdump_Tensor_vec_ref_t tensor_vec_ref = etdump_Tensor_vec_end(builder_);
579601
etdump_TensorList_ref_t tensor_list_ref =
@@ -645,6 +667,7 @@ void ETDumpGen::log_evalue(const EValue& evalue, LoggedEValueType evalue_type) {
645667
etdump_RunData_events_push_start(builder_);
646668
etdump_Event_debug_event_add(builder_, debug_event);
647669
etdump_RunData_events_push_end(builder_);
670+
return true;
648671
}
649672

650673
size_t ETDumpGen::get_num_blocks() {

devtools/etdump/etdump_flatcc.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,7 @@ class ETDumpGen : public ::executorch::runtime::EventTracer {
102102
size_t size) override;
103103
virtual ::executorch::runtime::AllocatorID track_allocator(
104104
const char* name) override;
105-
virtual void log_evalue(
105+
virtual Result<bool> log_evalue(
106106
const ::executorch::runtime::EValue& evalue,
107107
::executorch::runtime::LoggedEValueType evalue_type =
108108
::executorch::runtime::LoggedEValueType::kIntermediateOutput)

runtime/core/event_tracer.h

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -313,8 +313,11 @@ class EventTracer {
313313
* @param[in] evalue The value to be logged.
314314
* @param[in] evalue_type Indicates what type of output this is logging e.g.
315315
* an intermediate output, program output etc.
316+
* @return A Result<bool> indicating the status of the logging operation.
317+
* - True if the evalue output was successfully logged.
318+
* - An error code if an error occurs during logging.
316319
*/
317-
virtual void log_evalue(
320+
virtual Result<bool> log_evalue(
318321
const EValue& evalue,
319322
LoggedEValueType evalue_type) = 0;
320323

runtime/core/test/event_tracer_test.cpp

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -161,9 +161,11 @@ class DummyEventTracer : public EventTracer {
161161
return true;
162162
}
163163

164-
void log_evalue(const EValue& evalue, LoggedEValueType evalue_type) override {
164+
Result<bool> log_evalue(const EValue& evalue, LoggedEValueType evalue_type)
165+
override {
165166
logged_evalue_ = evalue;
166167
logged_evalue_type_ = evalue_type;
168+
return true;
167169
}
168170

169171
EValue logged_evalue() {

0 commit comments

Comments
 (0)