Skip to content

Commit bf838aa

Browse files
author
zhenyanzhang
committed
[#9971] Gracefully error out in ETDump for get_flatbuffer_scalar_type
Pull Request resolved: #10056 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> ghstack-source-id: 277252745 @exported-using-ghexport Differential Revision: [D72771753](https://our.internmc.facebook.com/intern/diff/D72771753/)
1 parent 1facfa9 commit bf838aa

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()) {
@@ -398,18 +403,26 @@ Result<bool> ETDumpGen::log_intermediate_output_delegate_helper(
398403
// Check the type of `output` then call the corresponding logging functions
399404
if constexpr (std::is_same<T, Tensor>::value) {
400405
long offset = write_tensor_or_raise_error(output);
401-
etdump_Tensor_ref_t tensor_ref = add_tensor_entry(builder_, output, offset);
406+
Result<etdump_Tensor_ref_t> tensor_ref =
407+
add_tensor_entry(builder_, output, offset);
408+
if (!tensor_ref.ok()) {
409+
return tensor_ref.error();
410+
}
402411

403412
etdump_Value_start(builder_);
404413
etdump_Value_val_add(builder_, etdump_ValueType_Tensor);
405-
etdump_Value_tensor_add(builder_, tensor_ref);
414+
etdump_Value_tensor_add(builder_, tensor_ref.get());
406415

407416
} else if constexpr (std::is_same<T, ArrayRef<Tensor>>::value) {
408417
etdump_Tensor_vec_start(builder_);
409418
for (size_t i = 0; i < output.size(); ++i) {
410419
long offset = write_tensor_or_raise_error(output[i]);
411-
etdump_Tensor_vec_push(
412-
builder_, add_tensor_entry(builder_, output[i], offset));
420+
Result<etdump_Tensor_ref_t> tensor_ref =
421+
add_tensor_entry(builder_, output[i], offset);
422+
if (!tensor_ref.ok()) {
423+
return tensor_ref.error();
424+
}
425+
etdump_Tensor_vec_push(builder_, tensor_ref.get());
413426
}
414427
etdump_Tensor_vec_ref_t tensor_vec_ref = etdump_Tensor_vec_end(builder_);
415428
etdump_TensorList_ref_t tensor_list_ref =
@@ -546,7 +559,9 @@ void ETDumpGen::set_data_sink(DataSinkBase* data_sink) {
546559
data_sink_ = data_sink;
547560
}
548561

549-
void ETDumpGen::log_evalue(const EValue& evalue, LoggedEValueType evalue_type) {
562+
Result<bool> ETDumpGen::log_evalue(
563+
const EValue& evalue,
564+
LoggedEValueType evalue_type) {
550565
check_ready_to_add_events();
551566

552567
etdump_DebugEvent_start(builder_);
@@ -558,12 +573,15 @@ void ETDumpGen::log_evalue(const EValue& evalue, LoggedEValueType evalue_type) {
558573
case Tag::Tensor: {
559574
executorch::aten::Tensor tensor = evalue.toTensor();
560575
long offset = write_tensor_or_raise_error(tensor);
561-
etdump_Tensor_ref_t tensor_ref =
576+
Result<etdump_Tensor_ref_t> tensor_ref =
562577
add_tensor_entry(builder_, tensor, offset);
578+
if (!tensor_ref.ok()) {
579+
return tensor_ref.error();
580+
}
563581

564582
etdump_Value_start(builder_);
565583
etdump_Value_val_add(builder_, etdump_ValueType_Tensor);
566-
etdump_Value_tensor_add(builder_, tensor_ref);
584+
etdump_Value_tensor_add(builder_, tensor_ref.get());
567585
if (evalue_type == LoggedEValueType::kProgramOutput) {
568586
auto bool_ref = etdump_Bool_create(builder_, FLATBUFFERS_TRUE);
569587
etdump_Value_output_add(builder_, bool_ref);
@@ -580,8 +598,12 @@ void ETDumpGen::log_evalue(const EValue& evalue, LoggedEValueType evalue_type) {
580598
etdump_Tensor_vec_start(builder_);
581599
for (size_t i = 0; i < tensors.size(); ++i) {
582600
long offset = write_tensor_or_raise_error(tensors[i]);
583-
etdump_Tensor_vec_push(
584-
builder_, add_tensor_entry(builder_, tensors[i], offset));
601+
Result<etdump_Tensor_ref_t> tensor_ref =
602+
add_tensor_entry(builder_, tensors[i], offset);
603+
if (!tensor_ref.ok()) {
604+
return tensor_ref.error();
605+
}
606+
etdump_Tensor_vec_push(builder_, tensor_ref.get());
585607
}
586608
etdump_Tensor_vec_ref_t tensor_vec_ref = etdump_Tensor_vec_end(builder_);
587609
etdump_TensorList_ref_t tensor_list_ref =
@@ -653,6 +675,7 @@ void ETDumpGen::log_evalue(const EValue& evalue, LoggedEValueType evalue_type) {
653675
etdump_RunData_events_push_start(builder_);
654676
etdump_Event_debug_event_add(builder_, debug_event);
655677
etdump_RunData_events_push_end(builder_);
678+
return true;
656679
}
657680

658681
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)