Skip to content

Commit d767516

Browse files
authored
Move output tensor resize to before XNNPACK subgraph execution (#17661)
Summary: ET's XNNPACK delegate currently resizes output tensors after running the XNNPACK subgraph. This is normally fine, as the buffer is large enough. However, if there is a logic bug or corrupt model file, the current code doesn't catch if XNNPACK's output is larger than the planned buffer until after execution, where it may crash. To fix this, I've moved the resize up. This fails gracefully in the above case. Differential Revision: D94148443
1 parent c65d8da commit d767516

4 files changed

Lines changed: 37 additions & 14 deletions

File tree

backends/xnnpack/runtime/XNNExecutor.cpp

Lines changed: 24 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -127,7 +127,7 @@ ET_NODISCARD Error XNNExecutor::prepare_args(Span<EValue*> args) {
127127
xnn_status_to_string(status));
128128
}
129129
}
130-
// // Propagate Input Shape and Memory Plan for increased allocation
130+
// Propagate Input Shape and Memory Plan for increased allocation
131131
status = xnn_reshape_runtime(runtime_.get());
132132

133133
ET_CHECK_OR_RETURN_ERROR(
@@ -136,6 +136,12 @@ ET_NODISCARD Error XNNExecutor::prepare_args(Span<EValue*> args) {
136136
"Internal Error: Propagating input shapes failed with code: %s",
137137
xnn_status_to_string(status));
138138

139+
// Resize output tensors.
140+
Error err = resize_outputs(args);
141+
if (err != Error::Ok) {
142+
return err;
143+
}
144+
139145
return Error::Ok;
140146
}
141147

@@ -188,14 +194,7 @@ ET_NODISCARD Error XNNExecutor::forward(BackendExecutionContext& context) {
188194
}
189195

190196
/**
191-
* Prepares the outputs for ExecuTorch
192-
*
193-
* Resizes the output tensors based on the output shapes returned by
194-
* the xnnpack runtime.
195-
*
196-
* Note: For arg_max pooling, we recast the output index tensor. Since
197-
* XNNPACK gives the index tensor to us as int32, we need to convert it
198-
* back to int64 for ExecuTorch.
197+
* Resizes output tensors to match XNNPACK's computed shapes.
199198
*/
200199
ET_NODISCARD Error XNNExecutor::resize_outputs(Span<EValue*> args) const {
201200
size_t output_idx_start = input_ids_.size();
@@ -239,6 +238,22 @@ ET_NODISCARD Error XNNExecutor::resize_outputs(Span<EValue*> args) const {
239238
ET_LOG(Error, "Failed to resize output tensor for XNNExecutor");
240239
return err;
241240
}
241+
}
242+
243+
return Error::Ok;
244+
}
245+
246+
/**
247+
* Converts output data types after XNNPACK execution.
248+
*
249+
* For arg_max pooling, XNNPACK outputs int32 index tensors that need
250+
* to be converted to int64 for ExecuTorch.
251+
*/
252+
ET_NODISCARD Error XNNExecutor::convert_outputs(Span<EValue*> args) const {
253+
size_t output_idx_start = input_ids_.size();
254+
for (size_t i = output_idx_start; i < externals_.size(); ++i) {
255+
uint32_t ext_id = externals_[i].id;
256+
Tensor* out_tensor = &args[ext_id]->toTensor();
242257

243258
// Output datatype is int64. However, XNNPACK doesn't support
244259
// int64. This means that the data was put into this tensor

backends/xnnpack/runtime/XNNExecutor.h

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -88,13 +88,21 @@ class XNNExecutor {
8888
executorch::ET_RUNTIME_NAMESPACE::BackendExecutionContext& context);
8989

9090
/**
91-
* Prepares the outputs to be returned by the delegate
91+
* Resizes output tensors to match XNNPACK's computed shapes.
9292
*
93-
* Performs any post processing of outputs like tensor resizing
9493
*/
9594
ET_NODISCARD executorch::runtime::Error resize_outputs(
9695
executorch::runtime::Span<executorch::runtime::EValue*> args) const;
9796

97+
/**
98+
* Converts output data types after XNNPACK execution.
99+
*
100+
* For arg_max pooling, XNNPACK outputs int32 index tensors that need
101+
* to be converted to int64 for ExecuTorch.
102+
*/
103+
ET_NODISCARD executorch::runtime::Error convert_outputs(
104+
executorch::runtime::Span<executorch::runtime::EValue*> args) const;
105+
98106
friend class XNNCompiler;
99107
};
100108

backends/xnnpack/runtime/XNNPACKBackend.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -152,8 +152,8 @@ class XnnpackBackend final
152152
return err;
153153
}
154154

155-
// Resize outputs and recast pointers if necessary
156-
err = executor->resize_outputs(args);
155+
// Convert output data types if necessary (e.g., int32 -> int64 for Long)
156+
err = executor->convert_outputs(args);
157157

158158
return err;
159159
}

backends/xnnpack/test/runtime/test_xnnexecutor.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -174,7 +174,7 @@ TEST(XNNExecutorTest, ResizeOutputsWithLongTensorConvertsInt32ToInt64) {
174174
ASSERT_EQ(executor.prepare_args(span), Error::Ok);
175175
executorch::ET_RUNTIME_NAMESPACE::BackendExecutionContext context;
176176
ASSERT_EQ(executor.forward(context), Error::Ok);
177-
ASSERT_EQ(executor.resize_outputs(span), Error::Ok);
177+
ASSERT_EQ(executor.convert_outputs(span), Error::Ok);
178178

179179
Tensor& result = args[2]->toTensor();
180180
ASSERT_EQ(result.scalar_type(), executorch::aten::ScalarType::Long);

0 commit comments

Comments
 (0)