Move tensor_shape_to_c_string back to runtime/core

I moved it to kernels/portable due to build issues, but it doesn't make sense that kernels/portable code can't use this if it's in runtime/core, so let's dig in.

ghstack-source-id: 86ec3ad
ghstack-comment-id: 2641470188
Pull Request resolved: #8296

Author: swolchok

kernels/portable/cpu/util/broadcast_util.cpp

@@ -7,9 +7,9 @@
  */
 
 #include <executorch/kernels/portable/cpu/util/repeat_util.h>
-#include <executorch/kernels/portable/cpu/util/tensor_util.h>
 #include <executorch/runtime/core/exec_aten/exec_aten.h>
 #include <executorch/runtime/core/exec_aten/util/scalar_type_util.h>
+#include <executorch/runtime/core/exec_aten/util/tensor_shape_to_c_string.h>
 #include <string.h>
 
 namespace torch {

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,17 +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/core/exec_aten:lib",
-            "//executorch/runtime/kernel:kernel_includes",
-        ],
-        visibility = ["//executorch/kernels/portable/cpu/..."],
-    )
-
     runtime.cxx_library(
         name = "upsample_util",
         srcs = ["upsample_util.cpp"],

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

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",
-        ],
-    )

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 <class SizesType>
-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<size_t>(sizes[i]));
-    auto len = strlen(output);
-    output += len;
-    remaining_size -= len;
-  }
-  snprintf(output, remaining_size, ")");
-}
-
 } // namespace runtime
 } // namespace executorch
 

runtime/core/exec_aten/util/targets.bzl

@@ -72,3 +72,26 @@ def define_common_targets():
             # specify library directory path.
             force_static = True,
         )
+
+        runtime.cxx_library(
+            name = "tensor_shape_to_c_string" + aten_suffix,
+            srcs = ["tensor_shape_to_c_string.cpp"],
+            exported_deps = [
+                "//executorch/runtime/core:core",
+                "//executorch/runtime/core/exec_aten/util:tensor_dimension_limit",
+            ],
+            exported_headers = ["tensor_shape_to_c_string.h"],
+            visibility = [
+                "//executorch/...",
+                "@EXECUTORCH_CLIENTS",
+            ],
+        )
+
+    runtime.cxx_library(
+        name = "tensor_dimension_limit",
+        exported_headers = ["tensor_dimension_limit.h"],
+        visibility = [
+            "//executorch/...",
+            "@EXECUTORCH_CLIENTS",
+        ],
+    )

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

kernels/portable/cpu/util/tensor_util.cpp renamed to runtime/core/exec_aten/util/tensor_shape_to_c_string.cpp

@@ -6,17 +6,19 @@
  * LICENSE file in the root directory of this source tree.
  */
 
-#include <executorch/kernels/portable/cpu/util/tensor_util.h>
-#include <executorch/runtime/core/exec_aten/util/tensor_util.h>
+#include <executorch/runtime/core/exec_aten/util/tensor_shape_to_c_string.h>
+
 #include <executorch/runtime/platform/assert.h>
 
+#include <cinttypes>
+#include <cstdio> // For snprintf.
+#include <cstring>
+
 namespace executorch::runtime {
-/**
- * Shared implementation for tensor_util.h, may only contain code that
- * works whether or not ATen mode is active.
- */
-std::array<char, kTensorShapeStringSizeLimit> tensor_shape_to_c_string(
-    executorch::runtime::Span<const executorch::aten::SizesType> shape) {
+namespace {
+template <typename SizesType>
+std::array<char, kTensorShapeStringSizeLimit> tensor_shape_to_c_string_impl(
+    executorch::runtime::Span<SizesType> shape) {
   std::array<char, kTensorShapeStringSizeLimit> out;
   char* p = out.data();
   if ET_UNLIKELY (shape.size() > kTensorDimensionLimit) {
@@ -48,5 +50,16 @@ std::array<char, kTensorShapeStringSizeLimit> tensor_shape_to_c_string(
   *(p - 1) = '\0';
   return out;
 }
+} // namespace
+
+std::array<char, kTensorShapeStringSizeLimit> tensor_shape_to_c_string(
+    executorch::runtime::Span<const std::int32_t> shape) {
+  return tensor_shape_to_c_string_impl(shape);
+}
+
+std::array<char, kTensorShapeStringSizeLimit> tensor_shape_to_c_string(
+    executorch::runtime::Span<const std::int64_t> shape) {
+  return tensor_shape_to_c_string_impl(shape);
+}
 
 } // namespace executorch::runtime

kernels/portable/cpu/util/tensor_util.h renamed to runtime/core/exec_aten/util/tensor_shape_to_c_string.h

@@ -10,10 +10,10 @@
 
 #include <array>
 #include <cstddef>
+#include <cstdint>
 #include <limits>
 
-#include <executorch/runtime/core/exec_aten/exec_aten.h>
-#include <executorch/runtime/core/exec_aten/util/tensor_util.h>
+#include <executorch/runtime/core/exec_aten/util/tensor_dimension_limit.h>
 #include <executorch/runtime/core/span.h>
 
 namespace executorch::runtime {
@@ -34,18 +34,35 @@ constexpr size_t kTensorShapeStringSizeLimit = 1 + /* opening parenthesis */
 
 namespace internal {
 constexpr size_t kMaximumPrintableTensorShapeElement =
-    std::is_same_v<executorch::aten::SizesType, int32_t>
-    ? std::numeric_limits<int32_t>::max()
-    : std::numeric_limits<uint32_t>::max();
+    std::numeric_limits<int32_t>::max();
 } // namespace internal
 
 /**
  * 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<char, kTensorShapeStringSizeLimit> tensor_shape_to_c_string(
+    executorch::runtime::Span<const std::int32_t> 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<char, kTensorShapeStringSizeLimit> tensor_shape_to_c_string(
-    executorch::runtime::Span<const executorch::aten::SizesType> shape);
+    executorch::runtime::Span<const std::int64_t> shape);
 
 } // namespace executorch::runtime

runtime/core/exec_aten/util/tensor_util.h

@@ -20,6 +20,7 @@
 #include <executorch/runtime/core/exec_aten/exec_aten.h>
 #include <executorch/runtime/core/exec_aten/util/dim_order_util.h>
 #include <executorch/runtime/core/exec_aten/util/scalar_type_util.h>
+#include <executorch/runtime/core/exec_aten/util/tensor_dimension_limit.h>
 #include <executorch/runtime/core/span.h>
 #include <executorch/runtime/platform/assert.h>
 #include <executorch/runtime/platform/compiler.h>
@@ -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,