From 0a0488377a29c3f881d2e9be0abbdeb064595394 Mon Sep 17 00:00:00 2001 From: Scott Wolchok Date: Fri, 24 Jan 2025 10:57:04 -0800 Subject: [PATCH 1/9] Update [ghstack-poisoned] --- runtime/core/array_ref.h | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/runtime/core/array_ref.h b/runtime/core/array_ref.h index 463284d9c68..d02aac955ce 100644 --- a/runtime/core/array_ref.h +++ b/runtime/core/array_ref.h @@ -19,7 +19,6 @@ // removed some implicit const -> non-const conversions that rely on // complicated std::enable_if meta-programming // removed a bunch of slice variants for simplicity... -// remove constructors for std::array // remove constructors and operators for std::vector // removed some prevention of accidental assignments from temporary that // required std::enable_if meta-programming @@ -27,6 +26,7 @@ #pragma once +#include #include #include @@ -87,6 +87,11 @@ class ArrayRef final { /// Construct a ArrayRef from a range. ArrayRef(const T* begin, const T* end) : Data(begin), Length(end - begin) {} + /// Construct an ArrayRef from a std::array + template + /* implicit */ constexpr ArrayRef(const std::array& Arr) + : Data(Arr.data()), Length(N) {} + /// Construct a ArrayRef from a C array. template /* implicit */ constexpr ArrayRef(const T (&Arr)[N]) : Data(Arr), Length(N) {} @@ -202,6 +207,12 @@ ArrayRef makeArrayRef(const T* begin, const T* end) { return ArrayRef(begin, end); } +/// Construct an ArrayRef from a std::array. +template +ArrayRef makeArrayRef(const std::array& Arr) { + return Arr; +} + /// Construct an ArrayRef from an ArrayRef (no-op) (const) template ArrayRef makeArrayRef(const ArrayRef& Vec) { From 7ebb45fe39b7c71d21ece7ed376113b19bed1c0d Mon Sep 17 00:00:00 2001 From: Scott Wolchok Date: Fri, 24 Jan 2025 10:57:09 -0800 Subject: [PATCH 2/9] Update [ghstack-poisoned] --- runtime/core/exec_aten/util/targets.bzl | 2 +- runtime/core/exec_aten/util/tensor_util.cpp | 43 +++++++++++++++++++ runtime/core/exec_aten/util/tensor_util.h | 23 ++++++++++ .../exec_aten/util/test/tensor_util_test.cpp | 43 +++++++++++++++++++ 4 files changed, 110 insertions(+), 1 deletion(-) create mode 100644 runtime/core/exec_aten/util/tensor_util.cpp diff --git a/runtime/core/exec_aten/util/targets.bzl b/runtime/core/exec_aten/util/targets.bzl index e66ffa3a028..9cdead37883 100644 --- a/runtime/core/exec_aten/util/targets.bzl +++ b/runtime/core/exec_aten/util/targets.bzl @@ -50,7 +50,7 @@ def define_common_targets(): runtime.cxx_library( name = "tensor_util" + aten_suffix, - srcs = ["tensor_util_aten.cpp"] if aten_mode else ["tensor_util_portable.cpp"], + srcs = ["tensor_util.cpp"] + (["tensor_util_aten.cpp"] if aten_mode else ["tensor_util_portable.cpp"]), exported_headers = [ "tensor_util.h", ], diff --git a/runtime/core/exec_aten/util/tensor_util.cpp b/runtime/core/exec_aten/util/tensor_util.cpp new file mode 100644 index 00000000000..e99a7d080ef --- /dev/null +++ b/runtime/core/exec_aten/util/tensor_util.cpp @@ -0,0 +1,43 @@ +#include "tensor_util.h" + +/* + * Copyright (c) Meta Platforms, Inc. and affiliates. + * All rights reserved. + * + * This source code is licensed under the BSD-style license found in the + * LICENSE file in the root directory of this source tree. + */ + +#include + +#include + +namespace executorch::runtime { +/** + * Shared implementation for tensor_util.h, may only contain code that + * works whether or not ATen mode is active. + */ +void tensor_shape_to_c_string( + char out[kTensorShapeStringSizeLimit], + executorch::aten::ArrayRef shape) { + char* p = out; + *p++ = '('; + for (const auto elem : shape) { + if (elem < 0 || elem > kMaximumPrintableTensorShapeElement) { + strcpy(p, "ERR, "); + p += strlen("ERR, "); + } else { + // snprintf returns characters *except* the NUL terminator, which is what + // we want. + p += snprintf( + p, + kTensorShapeStringSizeLimit - (p - out), + "%" PRIu32 ", ", + static_cast(elem)); + } + } + *(p - 2) = ')'; + *(p - 1) = '\0'; +} + +} // namespace executorch::runtime diff --git a/runtime/core/exec_aten/util/tensor_util.h b/runtime/core/exec_aten/util/tensor_util.h index 6fdc1bc2936..0469efcfbac 100644 --- a/runtime/core/exec_aten/util/tensor_util.h +++ b/runtime/core/exec_aten/util/tensor_util.h @@ -1120,6 +1120,29 @@ bool extract_scalar_tensor(executorch::aten::Tensor tensor, BOOL_T* out_val) { return true; } +/** + * Maximum size of a string returned by tensor_shape_to_c_string, for + * stack allocation. + */ +constexpr size_t kTensorShapeStringSizeLimit = 1 + /* opening parenthesis */ + 10 * kTensorDimensionLimit + /* maximum digits we will print; update + * kMaximumPrintableTensorShapeElement + * if changing */ + 2 * kTensorDimensionLimit + /* comma and space after each item, + * overwritten with closing paren and + * NUL terminator for last element */ + 1; /* padding for temporary NUL terminator for simplicity of implementation + */ + +constexpr size_t kMaximumPrintableTensorShapeElement = + std::is_same_v + ? std::numeric_limits::max() + : std::numeric_limits::max(); + +void tensor_shape_to_c_string( + char out[kTensorShapeStringSizeLimit], + executorch::aten::ArrayRef shape); + /// These APIs should not be used outside of Executor.cpp. namespace internal { /** diff --git a/runtime/core/exec_aten/util/test/tensor_util_test.cpp b/runtime/core/exec_aten/util/test/tensor_util_test.cpp index 88588dade68..1a0db784147 100644 --- a/runtime/core/exec_aten/util/test/tensor_util_test.cpp +++ b/runtime/core/exec_aten/util/test/tensor_util_test.cpp @@ -12,6 +12,7 @@ #include #include #include +#include #include using namespace ::testing; @@ -605,3 +606,45 @@ TEST_F(TensorUtilTest, SameShapesDifferentDimOrder) { EXPECT_FALSE(tensors_have_same_dim_order(a, c, b)); EXPECT_FALSE(tensors_have_same_dim_order(c, b, a)); } + +TEST_F(TensorUtilTest, TensorShapeToCStringBasic) { + char str[executorch::runtime::kTensorShapeStringSizeLimit]; + std::array sizes = {123, 456, 789}; + executorch::runtime::tensor_shape_to_c_string(str, sizes); + EXPECT_STREQ(str, "(123, 456, 789)"); + + std::array one_size = {1234567890}; + executorch::runtime::tensor_shape_to_c_string(str, one_size); + EXPECT_STREQ(str, "(1234567890)"); +} + +TEST_F(TensorUtilTest, TensorShapeToCStringMaximumLength) { + using executorch::runtime::kMaximumPrintableTensorShapeElement; + using executorch::runtime::kTensorDimensionLimit; + using executorch::runtime::kTensorShapeStringSizeLimit; + using executorch::runtime::tensor_shape_to_c_string; + char str[executorch::runtime::kTensorShapeStringSizeLimit + 1]; + std::memset(str, '@', sizeof(str)); + + std::array< + executorch::aten::SizesType, + executorch::runtime::kTensorDimensionLimit> + sizes; + std::fill( + sizes.begin(), + sizes.end(), + executorch::runtime::kMaximumPrintableTensorShapeElement); + + executorch::runtime::tensor_shape_to_c_string(str, sizes); + + std::ostringstream expected; + expected << '(' << kMaximumPrintableTensorShapeElement; + for (int ii = 0; ii < kTensorDimensionLimit - 1; ++ii) { + expected << ", " << kMaximumPrintableTensorShapeElement; + } + expected << ')'; + auto expected_str = expected.str(); + + EXPECT_EQ(str[executorch::runtime::kTensorShapeStringSizeLimit], '@'); + EXPECT_EQ(expected_str, str); +} From c27d91dff6cd5a5abeb33cc6e4f7bbc93d0e046e Mon Sep 17 00:00:00 2001 From: Scott Wolchok Date: Fri, 24 Jan 2025 10:57:13 -0800 Subject: [PATCH 3/9] Update [ghstack-poisoned] --- kernels/portable/cpu/util/broadcast_util.cpp | 16 ++++++++++++---- .../portable/cpu/util/test/broadcast_test.cpp | 9 +++++++++ 2 files changed, 21 insertions(+), 4 deletions(-) diff --git a/kernels/portable/cpu/util/broadcast_util.cpp b/kernels/portable/cpu/util/broadcast_util.cpp index 943219490b0..9d6599e9fce 100644 --- a/kernels/portable/cpu/util/broadcast_util.cpp +++ b/kernels/portable/cpu/util/broadcast_util.cpp @@ -213,10 +213,18 @@ ET_NODISCARD Error get_broadcast_target_size( Tensor::SizesType* out_sizes, const size_t out_sizes_len, size_t* out_dim) { - ET_CHECK_OR_RETURN_ERROR( - tensors_are_broadcastable_between(a_size, b_size), - InvalidArgument, - "Two input tensors should be broadcastable.\n"); + if (!tensors_are_broadcastable_between(a_size, b_size)) { + char a_shape_str[executorch::runtime::kTensorShapeStringSizeLimit]; + char b_shape_str[executorch::runtime::kTensorShapeStringSizeLimit]; + tensor_shape_to_c_string(a_shape_str, a_size); + tensor_shape_to_c_string(b_shape_str, b_size); + ET_LOG( + Error, + "Two input tensors should be broadcastable but got shapes %s and %s.", + a_shape_str, + b_shape_str); + return executorch::runtime::Error::InvalidArgument; + } auto a_dim = a_size.size(); auto b_dim = b_size.size(); diff --git a/kernels/portable/cpu/util/test/broadcast_test.cpp b/kernels/portable/cpu/util/test/broadcast_test.cpp index d87e8ecec85..954525b9490 100644 --- a/kernels/portable/cpu/util/test/broadcast_test.cpp +++ b/kernels/portable/cpu/util/test/broadcast_test.cpp @@ -129,6 +129,15 @@ TEST(BroadcastUtilTest, GetBroadcastTargetSize) { EXPECT_TRUE( ArrayRef(expected_output_size, expected_output_dim) .equals(ArrayRef({5, 2, 2}))); + + Tensor c = tf.zeros({4, 5}); + err = get_broadcast_target_size( + a, + c, + expected_output_size, + torch::executor::kTensorDimensionLimit, + &expected_output_dim); + EXPECT_EQ(err, torch::executor::Error::InvalidArgument); } size_t linearize_indexes(size_t* indexes, size_t indexes_len, const Tensor& t) { From 5b97ef5afc9258d1fe3243e39ee79fde058f51e9 Mon Sep 17 00:00:00 2001 From: Scott Wolchok Date: Thu, 6 Feb 2025 16:15:57 -0800 Subject: [PATCH 4/9] Update [ghstack-poisoned] --- kernels/portable/cpu/util/broadcast_util.cpp | 2 +- kernels/portable/cpu/util/targets.bzl | 12 +----------- kernels/portable/cpu/util/test/CMakeLists.txt | 2 +- kernels/portable/cpu/util/test/targets.bzl | 8 -------- runtime/core/exec_aten/util/targets.bzl | 10 ++++++++++ .../core/exec_aten/util/tensor_shape_to_c_string.cpp | 0 .../core/exec_aten/util/tensor_shape_to_c_string.h | 0 runtime/core/exec_aten/util/test/CMakeLists.txt | 6 ++++-- runtime/core/exec_aten/util/test/targets.bzl | 8 ++++++++ .../util/test/tensor_shape_to_c_string_test.cpp | 12 ++++++------ test/utils/OSSTestConfig.json | 7 ++++--- 11 files changed, 35 insertions(+), 32 deletions(-) rename kernels/portable/cpu/util/tensor_util.cpp => runtime/core/exec_aten/util/tensor_shape_to_c_string.cpp (100%) rename kernels/portable/cpu/util/tensor_util.h => runtime/core/exec_aten/util/tensor_shape_to_c_string.h (100%) rename kernels/portable/cpu/util/test/tensor_util_test.cpp => runtime/core/exec_aten/util/test/tensor_shape_to_c_string_test.cpp (89%) diff --git a/kernels/portable/cpu/util/broadcast_util.cpp b/kernels/portable/cpu/util/broadcast_util.cpp index 38f41289378..f90d76932d4 100644 --- a/kernels/portable/cpu/util/broadcast_util.cpp +++ b/kernels/portable/cpu/util/broadcast_util.cpp @@ -7,9 +7,9 @@ */ #include -#include #include #include +#include #include namespace torch { diff --git a/kernels/portable/cpu/util/targets.bzl b/kernels/portable/cpu/util/targets.bzl index cdd3695cec7..0115feb6256 100644 --- a/kernels/portable/cpu/util/targets.bzl +++ b/kernels/portable/cpu/util/targets.bzl @@ -73,8 +73,8 @@ def define_common_targets(): compiler_flags = ["-Wno-missing-prototypes"], deps = [ ":repeat_util", - ":tensor_util", "//executorch/runtime/kernel:kernel_includes", + "//executorch/runtime/core/exec_aten/util:tensor_shape_to_c_string", "//executorch/runtime/core/exec_aten/util:tensor_util", ], visibility = ["//executorch/kernels/portable/cpu/...", "//executorch/kernels/optimized/cpu/...", "@EXECUTORCH_CLIENTS"], @@ -268,16 +268,6 @@ def define_common_targets(): visibility = ["//executorch/kernels/portable/cpu/..."], ) - runtime.cxx_library( - name = "tensor_util", - srcs = ["tensor_util.cpp"], - exported_headers = ["tensor_util.h"], - deps = [ - "//executorch/runtime/kernel:kernel_includes", - ], - visibility = ["//executorch/kernels/portable/cpu/..."], - ) - runtime.cxx_library( name = "upsample_util", srcs = ["upsample_util.cpp"], diff --git a/kernels/portable/cpu/util/test/CMakeLists.txt b/kernels/portable/cpu/util/test/CMakeLists.txt index bd18338da2f..5f81e4b6aec 100644 --- a/kernels/portable/cpu/util/test/CMakeLists.txt +++ b/kernels/portable/cpu/util/test/CMakeLists.txt @@ -19,7 +19,7 @@ set(EXECUTORCH_ROOT ${CMAKE_CURRENT_SOURCE_DIR}/../../../../..) include(${EXECUTORCH_ROOT}/build/Test.cmake) -set(_test_srcs broadcast_test.cpp reduce_test.cpp tensor_util_test.cpp) +set(_test_srcs broadcast_test.cpp reduce_test.cpp) et_cxx_test( kernels_portable_cpu_util_test SOURCES ${_test_srcs} EXTRA_LIBS diff --git a/kernels/portable/cpu/util/test/targets.bzl b/kernels/portable/cpu/util/test/targets.bzl index 0208f3488de..28988b90dcc 100644 --- a/kernels/portable/cpu/util/test/targets.bzl +++ b/kernels/portable/cpu/util/test/targets.bzl @@ -21,11 +21,3 @@ def define_common_targets(): "//executorch/kernels/portable/cpu/util:reduce_util", ], ) - - runtime.cxx_test( - name = "tensor_util_test", - srcs = ["tensor_util_test.cpp"], - deps = [ - "//executorch/kernels/portable/cpu/util:tensor_util", - ], - ) diff --git a/runtime/core/exec_aten/util/targets.bzl b/runtime/core/exec_aten/util/targets.bzl index e66ffa3a028..9514013a2d5 100644 --- a/runtime/core/exec_aten/util/targets.bzl +++ b/runtime/core/exec_aten/util/targets.bzl @@ -72,3 +72,13 @@ def define_common_targets(): # specify library directory path. force_static = True, ) + + runtime.cxx_library( + name = "tensor_shape_to_c_string", + srcs = ["tensor_shape_to_c_string.cpp"], + exported_headers = ["tensor_shape_to_c_string.h"], + visibility = [ + "//executorch/...", + "@EXECUTORCH_CLIENTS", + ], + ) diff --git a/kernels/portable/cpu/util/tensor_util.cpp b/runtime/core/exec_aten/util/tensor_shape_to_c_string.cpp similarity index 100% rename from kernels/portable/cpu/util/tensor_util.cpp rename to runtime/core/exec_aten/util/tensor_shape_to_c_string.cpp diff --git a/kernels/portable/cpu/util/tensor_util.h b/runtime/core/exec_aten/util/tensor_shape_to_c_string.h similarity index 100% rename from kernels/portable/cpu/util/tensor_util.h rename to runtime/core/exec_aten/util/tensor_shape_to_c_string.h diff --git a/runtime/core/exec_aten/util/test/CMakeLists.txt b/runtime/core/exec_aten/util/test/CMakeLists.txt index 86cdf26e68c..6123827e1ff 100644 --- a/runtime/core/exec_aten/util/test/CMakeLists.txt +++ b/runtime/core/exec_aten/util/test/CMakeLists.txt @@ -19,8 +19,10 @@ set(EXECUTORCH_ROOT ${CMAKE_CURRENT_SOURCE_DIR}/../../../../..) include(${EXECUTORCH_ROOT}/build/Test.cmake) -set(_test_srcs scalar_type_util_test.cpp - operator_impl_example_test.cpp dim_order_util_test.cpp +set(_test_srcs + dim_order_util_test.cpp operator_impl_example_test.cpp + scalar_type_util_test.cpp tensor_shape_to_c_string_test.cpp + tensor_util_test.cpp ) et_cxx_test(runtime_core_exec_aten_util_test SOURCES ${_test_srcs} EXTRA_LIBS) diff --git a/runtime/core/exec_aten/util/test/targets.bzl b/runtime/core/exec_aten/util/test/targets.bzl index 615b7c99a44..7d3114a8f82 100644 --- a/runtime/core/exec_aten/util/test/targets.bzl +++ b/runtime/core/exec_aten/util/test/targets.bzl @@ -46,3 +46,11 @@ def define_common_targets(): "//executorch/runtime/core/exec_aten/util:tensor_util" + aten_suffix, ], ) + + runtime.cxx_test( + name = "tensor_shape_to_c_string_test", + srcs = ["tensor_shape_to_c_string_test.cpp"], + deps = [ + "//executorch/runtime/core/exec_aten/util:tensor_shape_to_c_string", + ], + ) diff --git a/kernels/portable/cpu/util/test/tensor_util_test.cpp b/runtime/core/exec_aten/util/test/tensor_shape_to_c_string_test.cpp similarity index 89% rename from kernels/portable/cpu/util/test/tensor_util_test.cpp rename to runtime/core/exec_aten/util/test/tensor_shape_to_c_string_test.cpp index ee559e60311..8059360afd9 100644 --- a/kernels/portable/cpu/util/test/tensor_util_test.cpp +++ b/runtime/core/exec_aten/util/test/tensor_shape_to_c_string_test.cpp @@ -7,8 +7,8 @@ */ #include -#include #include +#include #include #include @@ -17,7 +17,7 @@ using executorch::runtime::Span; using executorch::runtime::tensor_shape_to_c_string; using executorch::runtime::internal::kMaximumPrintableTensorShapeElement; -TEST(TensorUtilTest, TensorShapeToCStringBasic) { +TEST(TensorShapeToCStringTest, Basic) { std::array sizes = {123, 456, 789}; auto str = tensor_shape_to_c_string( Span(sizes.data(), sizes.size())); @@ -29,7 +29,7 @@ TEST(TensorUtilTest, TensorShapeToCStringBasic) { EXPECT_STREQ(str.data(), "(1234567890)"); } -TEST(TensorUtilTest, TensorShapeToCStringNegativeItems) { +TEST(TensorShapeToCStringTest, NegativeItems) { std::array sizes = {-1, -3, -2, 4}; auto str = tensor_shape_to_c_string( Span(sizes.data(), sizes.size())); @@ -44,7 +44,7 @@ TEST(TensorUtilTest, TensorShapeToCStringNegativeItems) { EXPECT_EQ(str.data(), "(" + std::to_string(one_size[0]) + ")"); } } -TEST(TensorUtilTest, TensorShapeToCStringMaximumElement) { +TEST(TensorShapeToCStringTest, MaximumElement) { std::array sizes = { 123, std::numeric_limits::max(), 789}; auto str = tensor_shape_to_c_string( @@ -60,7 +60,7 @@ TEST(TensorUtilTest, TensorShapeToCStringMaximumElement) { EXPECT_EQ(str.data(), expected_str); } -TEST(TensorUtilTest, TensorShapeToCStringMaximumLength) { +TEST(TensorShapeToCStringTest, MaximumLength) { std::array sizes; std::fill(sizes.begin(), sizes.end(), kMaximumPrintableTensorShapeElement); @@ -78,7 +78,7 @@ TEST(TensorUtilTest, TensorShapeToCStringMaximumLength) { EXPECT_EQ(expected_str, str.data()); } -TEST(TensorUtilTest, TensorShapeToCStringExceedsDimensionLimit) { +TEST(TensorShapeToCStringTest, ExceedsDimensionLimit) { std::array sizes; std::fill(sizes.begin(), sizes.end(), kMaximumPrintableTensorShapeElement); diff --git a/test/utils/OSSTestConfig.json b/test/utils/OSSTestConfig.json index ce8c9723d23..2229b255401 100644 --- a/test/utils/OSSTestConfig.json +++ b/test/utils/OSSTestConfig.json @@ -85,10 +85,11 @@ { "directory": "runtime/core/exec_aten/util/test", "sources": [ - "tensor_util_test.cpp", - "scalar_type_util_test.cpp", + "dim_order_util_test.cpp", "operator_impl_example_test.cpp", - "dim_order_util_test.cpp" + "scalar_type_util_test.cpp", + "tensor_shape_to_c_string_test.cpp", + "tensor_util_test.cpp" ] }, { From c1b305e0715517f15434a4ffb87c36c1161e42bf Mon Sep 17 00:00:00 2001 From: Scott Wolchok Date: Thu, 6 Feb 2025 16:51:21 -0800 Subject: [PATCH 5/9] Update [ghstack-poisoned] --- runtime/core/exec_aten/util/dim_order_util.h | 29 ------------------- .../util/tensor_shape_to_c_string.cpp | 4 +-- runtime/core/portable_type/targets.bzl | 3 +- runtime/core/portable_type/tensor_impl.cpp | 19 +++++------- 4 files changed, 10 insertions(+), 45 deletions(-) diff --git a/runtime/core/exec_aten/util/dim_order_util.h b/runtime/core/exec_aten/util/dim_order_util.h index a1481ee3871..0aef3e5c6c9 100644 --- a/runtime/core/exec_aten/util/dim_order_util.h +++ b/runtime/core/exec_aten/util/dim_order_util.h @@ -260,35 +260,6 @@ ET_NODISCARD inline Error stride_to_dim_order( return Error::Ok; } -/** - * Print a string representation of an ArrayRef of tensor sizes into a - * user-provided string buffer. If the user buffer is too small, the string - * will be truncated. The output is of the format (1,2,3,4). - * - * Note that we cannot use ArrayRef here due to a circular dependency (see - * above comments). - */ -template -inline void sizes_to_string( - char* output, - size_t output_size, - SizesType* sizes, - size_t rank) { - auto remaining_size = output_size; - for (auto i = 0; remaining_size > 0 && i < rank; i++) { - snprintf( - output, - remaining_size, - "%s%zd", - i == 0 ? "(" : ",", - static_cast(sizes[i])); - auto len = strlen(output); - output += len; - remaining_size -= len; - } - snprintf(output, remaining_size, ")"); -} - } // namespace runtime } // namespace executorch diff --git a/runtime/core/exec_aten/util/tensor_shape_to_c_string.cpp b/runtime/core/exec_aten/util/tensor_shape_to_c_string.cpp index 51d60f82d81..1fee67e2d26 100644 --- a/runtime/core/exec_aten/util/tensor_shape_to_c_string.cpp +++ b/runtime/core/exec_aten/util/tensor_shape_to_c_string.cpp @@ -1,5 +1,3 @@ -#include "tensor_util.h" - /* * Copyright (c) Meta Platforms, Inc. and affiliates. * All rights reserved. @@ -8,7 +6,7 @@ * LICENSE file in the root directory of this source tree. */ -#include +#include #include diff --git a/runtime/core/portable_type/targets.bzl b/runtime/core/portable_type/targets.bzl index b8ccbe602ed..dfcd68c96de 100644 --- a/runtime/core/portable_type/targets.bzl +++ b/runtime/core/portable_type/targets.bzl @@ -32,8 +32,9 @@ def define_common_targets(): ":scalar_type", "//executorch/runtime/core:core", "//executorch/runtime/core:tensor_shape_dynamism", - "//executorch/runtime/core/exec_aten/util:scalar_type_util", "//executorch/runtime/core/exec_aten/util:dim_order_util", + "//executorch/runtime/core/exec_aten/util:scalar_type_util", + "//executorch/runtime/core/exec_aten/util:tensor_shape_to_c_string", "//executorch/runtime/core:tag", ], ) diff --git a/runtime/core/portable_type/tensor_impl.cpp b/runtime/core/portable_type/tensor_impl.cpp index 307210a465e..3c1bbf70eee 100644 --- a/runtime/core/portable_type/tensor_impl.cpp +++ b/runtime/core/portable_type/tensor_impl.cpp @@ -13,6 +13,7 @@ #include #include +#include #include #include #include @@ -95,18 +96,12 @@ Error TensorImpl::internal_resize_contiguous(ArrayRef new_sizes) { case TensorShapeDynamism::STATIC: if (!std::equal(sizes_, sizes_ + dim_, new_sizes.begin())) { #ifdef ET_LOG_ENABLED - std::array old_sizes_str, new_sizes_str; - - executorch::runtime::sizes_to_string( - old_sizes_str.data(), - old_sizes_str.size(), - sizes().data(), - sizes().size()); - executorch::runtime::sizes_to_string( - new_sizes_str.data(), - new_sizes_str.size(), - new_sizes.data(), - new_sizes.size()); + auto old_sizes_str = executorch::runtime::tensor_shape_to_c_string( + executorch::runtime::Span( + sizes().data(), sizes().size())); + auto new_sizes_str = executorch::runtime::tensor_shape_to_c_string( + executorch::runtime::Span( + new_sizes.data(), new_sizes.size())); #endif ET_CHECK_OR_RETURN_ERROR( From 2f996ba7e543c8d2558d930d12b17f99c97978ed Mon Sep 17 00:00:00 2001 From: Scott Wolchok Date: Thu, 6 Feb 2025 17:11:11 -0800 Subject: [PATCH 6/9] Update [ghstack-poisoned] --- runtime/core/exec_aten/util/targets.bzl | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/runtime/core/exec_aten/util/targets.bzl b/runtime/core/exec_aten/util/targets.bzl index 9514013a2d5..9379aeaa41c 100644 --- a/runtime/core/exec_aten/util/targets.bzl +++ b/runtime/core/exec_aten/util/targets.bzl @@ -73,12 +73,15 @@ def define_common_targets(): force_static = True, ) - runtime.cxx_library( - name = "tensor_shape_to_c_string", - srcs = ["tensor_shape_to_c_string.cpp"], - exported_headers = ["tensor_shape_to_c_string.h"], - visibility = [ - "//executorch/...", - "@EXECUTORCH_CLIENTS", - ], - ) + runtime.cxx_library( + name = "tensor_shape_to_c_string" + aten_suffix, + srcs = ["tensor_shape_to_c_string.cpp"], + exported_deps = [ + "//executorch/runtime/core/exec_aten:lib" + aten_suffix, + ], + exported_headers = ["tensor_shape_to_c_string.h"], + visibility = [ + "//executorch/...", + "@EXECUTORCH_CLIENTS", + ], + ) From b78fce9d5c035865747affde155c0970ffba1e49 Mon Sep 17 00:00:00 2001 From: Scott Wolchok Date: Thu, 6 Feb 2025 18:58:55 -0800 Subject: [PATCH 7/9] Update [ghstack-poisoned] --- runtime/core/exec_aten/util/targets.bzl | 12 +++++++- .../exec_aten/util/tensor_dimension_limit.h | 21 ++++++++++++++ .../util/tensor_shape_to_c_string.cpp | 23 +++++++++++---- .../exec_aten/util/tensor_shape_to_c_string.h | 29 +++++++++++++++---- runtime/core/exec_aten/util/tensor_util.h | 11 +------ runtime/core/portable_type/tensor_impl.cpp | 4 +-- 6 files changed, 75 insertions(+), 25 deletions(-) create mode 100644 runtime/core/exec_aten/util/tensor_dimension_limit.h diff --git a/runtime/core/exec_aten/util/targets.bzl b/runtime/core/exec_aten/util/targets.bzl index 9379aeaa41c..36e18930121 100644 --- a/runtime/core/exec_aten/util/targets.bzl +++ b/runtime/core/exec_aten/util/targets.bzl @@ -77,7 +77,8 @@ def define_common_targets(): name = "tensor_shape_to_c_string" + aten_suffix, srcs = ["tensor_shape_to_c_string.cpp"], exported_deps = [ - "//executorch/runtime/core/exec_aten:lib" + aten_suffix, + "//executorch/runtime/core:core", + "//executorch/runtime/core/exec_aten/util:tensor_dimension_limit", ], exported_headers = ["tensor_shape_to_c_string.h"], visibility = [ @@ -85,3 +86,12 @@ def define_common_targets(): "@EXECUTORCH_CLIENTS", ], ) + + runtime.cxx_library( + name = "tensor_dimension_limit", + exported_headers = ["tensor_dimension_limit.h"], + visibility = [ + "//executorch/...", + "@EXECUTORCH_CLIENTS", + ], + ) diff --git a/runtime/core/exec_aten/util/tensor_dimension_limit.h b/runtime/core/exec_aten/util/tensor_dimension_limit.h new file mode 100644 index 00000000000..6e072ab0582 --- /dev/null +++ b/runtime/core/exec_aten/util/tensor_dimension_limit.h @@ -0,0 +1,21 @@ +/* + * Copyright (c) Meta Platforms, Inc. and affiliates. + * All rights reserved. + * + * This source code is licensed under the BSD-style license found in the + * LICENSE file in the root directory of this source tree. + */ + +#pragma once + +namespace executorch::runtime { +/** + * The expected output size may not be the existing size of any inputs and + * outputs if the operator supports both broadcast and dynamic shape. + * Therefore such operators needs extra space to store the calculated expected + * output size. such dynamic allocation is troublesome in executorch so we can + * just hard code a static value of a relatively small value because users + * don't create high dimensional tensors. + */ +constexpr size_t kTensorDimensionLimit = 16; +} // namespace executorch::runtime diff --git a/runtime/core/exec_aten/util/tensor_shape_to_c_string.cpp b/runtime/core/exec_aten/util/tensor_shape_to_c_string.cpp index 1fee67e2d26..7612e3a372b 100644 --- a/runtime/core/exec_aten/util/tensor_shape_to_c_string.cpp +++ b/runtime/core/exec_aten/util/tensor_shape_to_c_string.cpp @@ -10,13 +10,13 @@ #include +#include + namespace executorch::runtime { -/** - * Shared implementation for tensor_util.h, may only contain code that - * works whether or not ATen mode is active. - */ -std::array tensor_shape_to_c_string( - executorch::runtime::Span shape) { +namespace { +template +std::array tensor_shape_to_c_string_impl( + executorch::runtime::Span shape) { std::array out; char* p = out.data(); if ET_UNLIKELY (shape.size() > kTensorDimensionLimit) { @@ -48,5 +48,16 @@ std::array tensor_shape_to_c_string( *(p - 1) = '\0'; return out; } +} // namespace + +std::array tensor_shape_to_c_string( + executorch::runtime::Span shape) { + return tensor_shape_to_c_string_impl(shape); +} + +std::array tensor_shape_to_c_string( + executorch::runtime::Span shape) { + return tensor_shape_to_c_string_impl(shape); +} } // namespace executorch::runtime diff --git a/runtime/core/exec_aten/util/tensor_shape_to_c_string.h b/runtime/core/exec_aten/util/tensor_shape_to_c_string.h index e3484f0e118..efc91f8b5d4 100644 --- a/runtime/core/exec_aten/util/tensor_shape_to_c_string.h +++ b/runtime/core/exec_aten/util/tensor_shape_to_c_string.h @@ -10,10 +10,10 @@ #include #include +#include #include -#include -#include +#include #include namespace executorch::runtime { @@ -34,9 +34,7 @@ constexpr size_t kTensorShapeStringSizeLimit = 1 + /* opening parenthesis */ namespace internal { constexpr size_t kMaximumPrintableTensorShapeElement = - std::is_same_v - ? std::numeric_limits::max() - : std::numeric_limits::max(); + std::numeric_limits::max(); } // namespace internal /** @@ -44,8 +42,27 @@ constexpr size_t kMaximumPrintableTensorShapeElement = * elements of the shape are larger than * kMaximumPrintableTensorShapeElement, those elements will be * rendered as ERR instead. + * + * NOTE: There are two overloads of this function to support both ATen + * tensors and ExecuTorch Tensors, which have different SizesType, + * while also avoiding a dependency on exec_aten.h from this header + * because that would cause a circular dependency. + */ +std::array tensor_shape_to_c_string( + executorch::runtime::Span shape); + +/** + * Convert a shape to a NUL-terminated C string with limited size. If + * elements of the shape are larger than + * kMaximumPrintableTensorShapeElement, those elements will be + * rendered as ERR instead. + * + * NOTE: There are two overloads of this function to support both ATen + * tensors and ExecuTorch Tensors, which have different SizesType, + * while also avoiding a dependency on exec_aten.h from this header + * because that would cause a circular dependency. */ std::array tensor_shape_to_c_string( - executorch::runtime::Span shape); + executorch::runtime::Span shape); } // namespace executorch::runtime diff --git a/runtime/core/exec_aten/util/tensor_util.h b/runtime/core/exec_aten/util/tensor_util.h index 1714bf53747..e16fe63e2a2 100644 --- a/runtime/core/exec_aten/util/tensor_util.h +++ b/runtime/core/exec_aten/util/tensor_util.h @@ -20,6 +20,7 @@ #include #include #include +#include #include #include #include @@ -893,16 +894,6 @@ inline bool tensor_is_scalar(executorch::aten::Tensor t) { return t.dim() == 0 && t.numel() == 1; } -/** - * The expected output size may not be the existing size of any inputs and - * outputs if the operator supports both broadcast and dynamic shape. - * Therefore such operators needs extra space to store the calculated expected - * output size. such dynamic allocation is troublesome in executorch so we can - * just hard code a static value of a relatively small value because users - * don't create high dimensional tensors. - */ -constexpr size_t kTensorDimensionLimit = 16; - /// Returns the product of dim[0:dim), not including dim. inline size_t getLeadingDims( const executorch::aten::Tensor& tensor, diff --git a/runtime/core/portable_type/tensor_impl.cpp b/runtime/core/portable_type/tensor_impl.cpp index 3c1bbf70eee..05e65375b66 100644 --- a/runtime/core/portable_type/tensor_impl.cpp +++ b/runtime/core/portable_type/tensor_impl.cpp @@ -97,10 +97,10 @@ Error TensorImpl::internal_resize_contiguous(ArrayRef new_sizes) { if (!std::equal(sizes_, sizes_ + dim_, new_sizes.begin())) { #ifdef ET_LOG_ENABLED auto old_sizes_str = executorch::runtime::tensor_shape_to_c_string( - executorch::runtime::Span( + executorch::runtime::Span( sizes().data(), sizes().size())); auto new_sizes_str = executorch::runtime::tensor_shape_to_c_string( - executorch::runtime::Span( + executorch::runtime::Span( new_sizes.data(), new_sizes.size())); #endif From 65970458f5ef8d8a4477cb4e2d2c9b9a962f695c Mon Sep 17 00:00:00 2001 From: Scott Wolchok Date: Fri, 7 Feb 2025 08:27:08 -0800 Subject: [PATCH 8/9] Update [ghstack-poisoned] --- runtime/core/exec_aten/util/tensor_shape_to_c_string.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/runtime/core/exec_aten/util/tensor_shape_to_c_string.cpp b/runtime/core/exec_aten/util/tensor_shape_to_c_string.cpp index 7612e3a372b..cfd416285c5 100644 --- a/runtime/core/exec_aten/util/tensor_shape_to_c_string.cpp +++ b/runtime/core/exec_aten/util/tensor_shape_to_c_string.cpp @@ -11,6 +11,8 @@ #include #include +#include // For snprintf. +#include namespace executorch::runtime { namespace { From f2538df1723366e14bfbe94b88c0e786ca54895a Mon Sep 17 00:00:00 2001 From: Scott Wolchok Date: Fri, 7 Feb 2025 10:18:15 -0800 Subject: [PATCH 9/9] Update [ghstack-poisoned] --- kernels/portable/cpu/util/broadcast_util.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/kernels/portable/cpu/util/broadcast_util.cpp b/kernels/portable/cpu/util/broadcast_util.cpp index 38f41289378..aca56ce7e97 100644 --- a/kernels/portable/cpu/util/broadcast_util.cpp +++ b/kernels/portable/cpu/util/broadcast_util.cpp @@ -213,13 +213,15 @@ ET_NODISCARD Error get_broadcast_target_size( Tensor::SizesType* out_sizes, const size_t out_sizes_len, size_t* out_dim) { - if (!tensors_are_broadcastable_between(a_size, b_size)) { + if ET_UNLIKELY (!tensors_are_broadcastable_between(a_size, b_size)) { +#ifdef ET_LOG_ENABLED const auto a_shape_str = tensor_shape_to_c_string( executorch::runtime::Span( a_size.data(), a_size.size())); const auto b_shape_str = tensor_shape_to_c_string( executorch::runtime::Span( b_size.data(), b_size.size())); +#endif ET_LOG( Error, "Two input tensors should be broadcastable but got shapes %s and %s.",