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: c98fe3d
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,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"],

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,16 @@ 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/exec_aten:lib" + aten_suffix,
+            ],
+            exported_headers = ["tensor_shape_to_c_string.h"],
+            visibility = [
+                "//executorch/...",
+                "@EXECUTORCH_CLIENTS",
+            ],
+        )

kernels/portable/cpu/util/tensor_util.cpp renamed to 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 <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>
 

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

@@ -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)

runtime/core/exec_aten/util/test/CMakeLists.txt

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

runtime/core/exec_aten/util/test/targets.bzl

@@ -7,8 +7,8 @@
  */
 #include <gtest/gtest.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/tensor_shape_to_c_string.h>
 #include <executorch/runtime/platform/runtime.h>
 #include <array>
 
@@ -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<executorch::aten::SizesType, 3> sizes = {123, 456, 789};
   auto str = tensor_shape_to_c_string(
       Span<const executorch::aten::SizesType>(sizes.data(), sizes.size()));
@@ -29,7 +29,7 @@ TEST(TensorUtilTest, TensorShapeToCStringBasic) {
   EXPECT_STREQ(str.data(), "(1234567890)");
 }
 
-TEST(TensorUtilTest, TensorShapeToCStringNegativeItems) {
+TEST(TensorShapeToCStringTest, NegativeItems) {
   std::array<executorch::aten::SizesType, 4> sizes = {-1, -3, -2, 4};
   auto str = tensor_shape_to_c_string(
       Span<const executorch::aten::SizesType>(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<executorch::aten::SizesType, 3> sizes = {
       123, std::numeric_limits<executorch::aten::SizesType>::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<executorch::aten::SizesType, kTensorDimensionLimit> 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<executorch::aten::SizesType, kTensorDimensionLimit + 1> sizes;
   std::fill(sizes.begin(), sizes.end(), kMaximumPrintableTensorShapeElement);
 

kernels/portable/cpu/util/test/tensor_util_test.cpp renamed to runtime/core/exec_aten/util/test/tensor_shape_to_c_string_test.cpp

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

runtime/core/portable_type/targets.bzl

@@ -13,6 +13,7 @@
 
 #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_shape_to_c_string.h>
 #include <executorch/runtime/core/portable_type/qint_types.h>
 #include <executorch/runtime/core/portable_type/scalar_type.h>
 #include <executorch/runtime/platform/assert.h>
@@ -95,18 +96,12 @@ Error TensorImpl::internal_resize_contiguous(ArrayRef<SizesType> new_sizes) {
     case TensorShapeDynamism::STATIC:
       if (!std::equal(sizes_, sizes_ + dim_, new_sizes.begin())) {
 #ifdef ET_LOG_ENABLED
-        std::array<char, 16> 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<const executorch::aten::SizesType>(
+                sizes().data(), sizes().size()));
+        auto new_sizes_str = executorch::runtime::tensor_shape_to_c_string(
+            executorch::runtime::Span<const executorch::aten::SizesType>(
+                new_sizes.data(), new_sizes.size()));
 #endif
 
         ET_CHECK_OR_RETURN_ERROR(

runtime/core/portable_type/tensor_impl.cpp

@@ -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"
         ]
     },
     {