Skip to content

[#9971] Gracefully error out in ETDump for get_flatbuffer_scalar_type #10056

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Apr 10, 2025
Merged

[#9971] Gracefully error out in ETDump for get_flatbuffer_scalar_type #10056

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6e72390
[#9971] Gracefully error out in ETDump for get_flatbuffer_scalar_type
Apr 10, 2025
d52cefd
Update on "[#9971] Gracefully error out in ETDump for get_flatbuffer_…
Apr 10, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
51 changes: 37 additions & 14 deletions devtools/etdump/etdump_flatcc.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ namespace executorch {
namespace etdump {
namespace {

executorch_flatbuffer_ScalarType_enum_t get_flatbuffer_scalar_type(
Result<executorch_flatbuffer_ScalarType_enum_t> get_flatbuffer_scalar_type(
executorch::aten::ScalarType tensor_scalar_type) {
switch (tensor_scalar_type) {
case executorch::aten::ScalarType::Byte:
Expand All @@ -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<char>(tensor_scalar_type));
}
}

etdump_Tensor_ref_t add_tensor_entry(
Result<etdump_Tensor_ref_t> 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<executorch_flatbuffer_ScalarType_enum_t> 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()) {
Expand Down Expand Up @@ -398,18 +403,26 @@ Result<bool> ETDumpGen::log_intermediate_output_delegate_helper(
// Check the type of `output` then call the corresponding logging functions
if constexpr (std::is_same<T, Tensor>::value) {
long offset = write_tensor_or_raise_error(output);
etdump_Tensor_ref_t tensor_ref = add_tensor_entry(builder_, output, offset);
Result<etdump_Tensor_ref_t> 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<T, ArrayRef<Tensor>>::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<etdump_Tensor_ref_t> 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 =
Expand Down Expand Up @@ -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<bool> ETDumpGen::log_evalue(
const EValue& evalue,
LoggedEValueType evalue_type) {
check_ready_to_add_events();

etdump_DebugEvent_start(builder_);
Expand All @@ -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<etdump_Tensor_ref_t> 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);
Expand All @@ -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<etdump_Tensor_ref_t> 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 =
Expand Down Expand Up @@ -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() {
Expand Down
2 changes: 1 addition & 1 deletion devtools/etdump/etdump_flatcc.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<bool> log_evalue(
const ::executorch::runtime::EValue& evalue,
::executorch::runtime::LoggedEValueType evalue_type =
::executorch::runtime::LoggedEValueType::kIntermediateOutput)
Expand Down
5 changes: 4 additions & 1 deletion runtime/core/event_tracer.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -313,8 +313,11 @@ 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<bool> 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 void log_evalue(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please update the comment for result value

virtual Result<bool> log_evalue(
const EValue& evalue,
LoggedEValueType evalue_type) = 0;

Expand Down
4 changes: 3 additions & 1 deletion runtime/core/test/event_tracer_test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -161,9 +161,11 @@ class DummyEventTracer : public EventTracer {
return true;
}

void log_evalue(const EValue& evalue, LoggedEValueType evalue_type) override {
Result<bool> log_evalue(const EValue& evalue, LoggedEValueType evalue_type)
override {
logged_evalue_ = evalue;
logged_evalue_type_ = evalue_type;
return true;
}

EValue logged_evalue() {
Expand Down
Loading