Skip to content

Commit 45ee5c1

Browse files
committed
Move output tensor resize to before XNNPACK subgraph execution (pytorch#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 3319157 commit 45ee5c1

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
@@ -84,13 +84,21 @@ class XNNExecutor {
8484
executorch::ET_RUNTIME_NAMESPACE::BackendExecutionContext& context);
8585

8686
/**
87-
* Prepares the outputs to be returned by the delegate
87+
* Resizes output tensors to match XNNPACK's computed shapes.
8888
*
89-
* Performs any post processing of outputs like tensor resizing
9089
*/
9190
ET_NODISCARD executorch::runtime::Error resize_outputs(
9291
executorch::runtime::Span<executorch::runtime::EValue*> args) const;
9392

93+
/**
94+
* Converts output data types after XNNPACK execution.
95+
*
96+
* For arg_max pooling, XNNPACK outputs int32 index tensors that need
97+
* to be converted to int64 for ExecuTorch.
98+
*/
99+
ET_NODISCARD executorch::runtime::Error convert_outputs(
100+
executorch::runtime::Span<executorch::runtime::EValue*> args) const;
101+
94102
friend class XNNCompiler;
95103
};
96104

backends/xnnpack/runtime/XNNPACKBackend.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -147,8 +147,8 @@ class XnnpackBackend final
147147
return err;
148148
}
149149

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

153153
return err;
154154
}

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)