Add function to stringify tensor shapes

[swolchok]

First step toward fixing #7902

[swolchok]

Stack from ghstack (oldest at bottom):

• Include tensor shapes in get_broadcast_target_size error message #7944
• -> Add function to stringify tensor shapes #7943

[pytorch-bot]

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/7943

• 📄 Preview Python docs built from this PR

Note: Links to docs will display an error until the docs builds have been completed.

❗ 1 Active SEVs

There are 1 currently active SEVs. If your PR is affected, please view them below:

• ROCM Infra failures during checkout of PyTorch

✅ No Failures

As of commit 61e79ce with merge base 62e49ce ():
💚 Looks good so far! There are no failures yet. 💚

This comment was automatically generated by Dr. CI and updates every 15 minutes.

[dbort]

I'm worried about the safety of this, since a caller could pass in any size despite the sized parameter:

% cat main.cpp
void test(char c[15]) {}

int main() {
  char b[7];
  test(b);
  return 0;
}

% clang++ --std=c++17 -Wall main.cpp
(no output; success)

A char* and size_t size or a Span<char> would make it more explicit.

Or if you wanted to stick with a fixed size array you could use std::array<char, kTensorShapeStringSizeLimit> which is enforced:

% cat main.cpp
#include <array>

void test(std::array<int, 15>c) {}

int main() {
  std::array<int, 7> b;
  test(b);
  return 0;
}


% clang++ --std=c++17 -Wall main.cpp
main.cpp:8:3: error: no matching function for call to 'test'
    8 |   test(b);
      |   ^~~~
main.cpp:3:6: note: candidate function not viable: no known conversion from 'array<[...], 7>' to 'array<[...], 15>' for 1st argument
    3 | void test(std::array<int, 15>c) {
      |      ^    ~~~~~~~~~~~~~~~~~~~~
1 error generated.

[dbort]

The new function signature looks great

[dbort]

We're trying to get away from ArrayRef for non-aten-compatible APIs; could you use executorch::runtime::Span for this?

[dbort]

Thanks for all of the changes here. I know I'm being paranoid but we really need to be extra paranoid in the core runtime.

A few final requests but looks good.

[dbort]

Could simplify things with snprintf()

And "can't happen" isn't true: I could create a PTE file where this does happen. Right now there's a dangerous assumption across the kernel libraries that dim is always <= kTensorDimensionLimit.

[dbort]

Took me a minute to understand where >99999 came from: please add a comment pointing out that it's the same number of digits as "ERR, " with the NUL

[dbort]

Should there be a -1 in here to account for the NUL byte? Not sure

[swolchok]

had to move the function into kernels/portable/cpu/util to fix an apparent layering violation -- portable_kernels failed to link because it needed to depend on executorch_core for this new function, but executorch_core already depends on portable_kernels

[swolchok]

apparent layering violation -- portable_kernels failed to link because it needed to depend on executorch_core for this new function, but executorch_core already depends on portable_kernels

I'll send a diff to move this back to runtime/core. portable_kernels really should be able to use code from runtime/core and either I was wrong or we should dig into why not.