From bf1ead9b001ab1cd52c5667ae1901816ecbf5fd3 Mon Sep 17 00:00:00 2001 From: Scott Wolchok Date: Fri, 7 Feb 2025 11:28:56 -0800 Subject: [PATCH 1/4] Update [ghstack-poisoned] --- .../portable/cpu/util/advanced_index_util.cpp | 20 ++++++++++++++++--- kernels/portable/cpu/util/targets.bzl | 1 + 2 files changed, 18 insertions(+), 3 deletions(-) diff --git a/kernels/portable/cpu/util/advanced_index_util.cpp b/kernels/portable/cpu/util/advanced_index_util.cpp index e2eabec4bc7..3022ec61b93 100644 --- a/kernels/portable/cpu/util/advanced_index_util.cpp +++ b/kernels/portable/cpu/util/advanced_index_util.cpp @@ -7,6 +7,7 @@ */ #include +#include #include namespace torch { @@ -49,9 +50,22 @@ bool check_mask_indices(const Tensor& in, TensorOptList indices) { ET_LOG_MSG_AND_RETURN_IF_FALSE( index.dim() > 0, "Zero-dimensional mask index not allowed"); for (auto j = 0; j < index.dim(); j++) { - ET_LOG_MSG_AND_RETURN_IF_FALSE( - index.size(j) == in.size(in_i + j), - "The shape of mask index must match the sizes of the corresponding input dimensions."); + if (index.size(j) != in.size(in_i + j)) { +#ifdef ET_LOG_ENABLED + auto mask_shape = executorch::runtime::tensor_shape_to_c_string( + executorch::runtime::Span( + index.sizes().data(), index.sizes().size())); + auto input_shape = executorch::runtime::tensor_shape_to_c_string( + executorch::runtime::Span( + in.sizes().data() + in_i, index.sizes().size())); + ET_LOG( + Error, + "The shape of mask index %s must match the sizes of the corresponding input dimensions %s.", + mask_shape.data(), + input_shape.data()); +#endif // ET_LOG_ENABLED + return false; + } } in_i += index.dim(); } else { diff --git a/kernels/portable/cpu/util/targets.bzl b/kernels/portable/cpu/util/targets.bzl index 0115feb6256..2c25d171568 100644 --- a/kernels/portable/cpu/util/targets.bzl +++ b/kernels/portable/cpu/util/targets.bzl @@ -117,6 +117,7 @@ def define_common_targets(): compiler_flags = ["-Wno-missing-prototypes"], deps = [ ":broadcast_util", + "//executorch/runtime/core/exec_aten/util:tensor_shape_to_c_string", "//executorch/runtime/kernel:kernel_includes", ], visibility = ["//executorch/kernels/portable/cpu/...", "//executorch/kernels/optimized/cpu/..."], From e7768b697a38ade4dc92b7da397b123fee1dae3f Mon Sep 17 00:00:00 2001 From: Scott Wolchok Date: Fri, 7 Feb 2025 11:35:44 -0800 Subject: [PATCH 2/4] Update [ghstack-poisoned] --- kernels/portable/cpu/util/advanced_index_util.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/kernels/portable/cpu/util/advanced_index_util.cpp b/kernels/portable/cpu/util/advanced_index_util.cpp index 3022ec61b93..6af310dbf63 100644 --- a/kernels/portable/cpu/util/advanced_index_util.cpp +++ b/kernels/portable/cpu/util/advanced_index_util.cpp @@ -58,12 +58,12 @@ bool check_mask_indices(const Tensor& in, TensorOptList indices) { auto input_shape = executorch::runtime::tensor_shape_to_c_string( executorch::runtime::Span( in.sizes().data() + in_i, index.sizes().size())); +#endif // ET_LOG_ENABLED ET_LOG( Error, "The shape of mask index %s must match the sizes of the corresponding input dimensions %s.", mask_shape.data(), input_shape.data()); -#endif // ET_LOG_ENABLED return false; } } From c8e02403c3932d8c4a5c14796ec25409766327fb Mon Sep 17 00:00:00 2001 From: Scott Wolchok Date: Fri, 7 Feb 2025 11:35:45 -0800 Subject: [PATCH 3/4] Update [ghstack-poisoned] --- .../portable/cpu/util/activation_ops_util.cpp | 20 ++++++++++++++++--- kernels/portable/cpu/util/targets.bzl | 1 + 2 files changed, 18 insertions(+), 3 deletions(-) diff --git a/kernels/portable/cpu/util/activation_ops_util.cpp b/kernels/portable/cpu/util/activation_ops_util.cpp index 195ffaf66d2..54a959c7821 100644 --- a/kernels/portable/cpu/util/activation_ops_util.cpp +++ b/kernels/portable/cpu/util/activation_ops_util.cpp @@ -9,6 +9,7 @@ #include #include +#include namespace torch { namespace executor { @@ -45,9 +46,22 @@ bool check_glu_args(const Tensor& in, int64_t dim, Tensor& out) { for (size_t i = 0; i < in.dim(); ++i) { if (i != non_negative_dim) { - ET_LOG_MSG_AND_RETURN_IF_FALSE( - out.size(i) == in.size(i), - "output tensor must have the same size as the input tensor in all dimensions except for the specified dimension."); + if (out.size(i) != in.size(i)) { +#ifdef ET_LOG_ENABLED + auto out_shape_str = executorch::runtime::tensor_shape_to_c_string( + executorch::runtime::Span( + out.sizes().data(), out.sizes().size())); + auto in_shape_str = executorch::runtime::tensor_shape_to_c_string( + executorch::runtime::Span( + in.sizes().data(), in.sizes().size())); +#endif // ET_LOG_ENABLED + ET_LOG( + Error, + "output tensor must have the same size as the input tensor in all dimensions except for the specified dimension. (output shape: %s input shape: %s)", + out_shape_str.data(), + in_shape_str.data()); + return false; + } } } diff --git a/kernels/portable/cpu/util/targets.bzl b/kernels/portable/cpu/util/targets.bzl index 2c25d171568..26f55a91e8d 100644 --- a/kernels/portable/cpu/util/targets.bzl +++ b/kernels/portable/cpu/util/targets.bzl @@ -44,6 +44,7 @@ def define_common_targets(): ], compiler_flags = ["-Wno-missing-prototypes"], deps = [ + "//executorch/runtime/core/exec_aten/util:tensor_shape_to_c_string", "//executorch/runtime/kernel:kernel_includes", ], visibility = ["//executorch/kernels/portable/cpu/...", "//executorch/kernels/optimized/cpu/..."], From 33ca14be3c72306912bd85f2b9c733795673e6b7 Mon Sep 17 00:00:00 2001 From: Scott Wolchok Date: Wed, 12 Feb 2025 20:29:00 -0800 Subject: [PATCH 4/4] Update [ghstack-poisoned] --- runtime/core/error.h | 16 ++++++++++++++ runtime/core/exec_aten/util/tensor_util.h | 27 ++++++----------------- 2 files changed, 23 insertions(+), 20 deletions(-) diff --git a/runtime/core/error.h b/runtime/core/error.h index cdf6303a650..7fbd92b7c08 100644 --- a/runtime/core/error.h +++ b/runtime/core/error.h @@ -126,6 +126,22 @@ using ::executorch::runtime::error_code_t; } \ } +/** + * A convenience macro to be used in utility functions that check whether input + * tensor(s) are valid, which are expected to return a boolean. Checks whether + * `cond` is true; if not, log the failed check with `message` and return false. + * + * @param[in] cond the condition to check + * @param[in] message an additional message to log with `cond` + */ +#define ET_CHECK_OR_RETURN_FALSE(cond__, message__, ...) \ + { \ + if (!(cond__)) { \ + ET_LOG(Error, "Check failed (%s): " message__, #cond__, ##__VA_ARGS__); \ + return false; \ + } \ + } + /** * If error__ is not Error::Ok, optionally log a message and return the error * from the current function, which must be of return type diff --git a/runtime/core/exec_aten/util/tensor_util.h b/runtime/core/exec_aten/util/tensor_util.h index e16fe63e2a2..d7edcfd21d5 100644 --- a/runtime/core/exec_aten/util/tensor_util.h +++ b/runtime/core/exec_aten/util/tensor_util.h @@ -332,35 +332,22 @@ }) /** + * DEPRECATED: Please use ET_CHECK_OR_RETURN_FALSE instead and provide + * an informative message. (For example, the values of any variables used in + * `cond` would not be reported automatically by this macro.) + * * A convenience macro to be used in utility functions that check whether input * tensor(s) are valid, which are expected to return a boolean. Checks whether * `cond` is true; if not, log the failed check and return false. * * @param[in] cond the condition to check */ -#define ET_LOG_AND_RETURN_IF_FALSE(cond) \ - do { \ - if (!(cond)) { \ - ET_LOG(Error, "Check failed (%s): ", #cond); \ - return false; \ - } \ - } while (false) +#define ET_LOG_AND_RETURN_IF_FALSE(cond) ET_CHECK_OR_RETURN_FALSE(cond, "") /** - * A convenience macro to be used in utility functions that check whether input - * tensor(s) are valid, which are expected to return a boolean. Checks whether - * `cond` is true; if not, log the failed check with `message` and return false. - * - * @param[in] cond the condition to check - * @param[in] message an additional message to log with `cond` + * DEPRECATED: Please use ET_CHECK_OR_RETURN_FALSE instead. */ -#define ET_LOG_MSG_AND_RETURN_IF_FALSE(cond, message, ...) \ - do { \ - if (!(cond)) { \ - ET_LOG(Error, "Check failed (%s): " message, #cond, ##__VA_ARGS__); \ - return false; \ - } \ - } while (false) +#define ET_LOG_MSG_AND_RETURN_IF_FALSE ET_CHECK_OR_RETURN_FALSE /** * If `cond` is false, log `cond` and return from the kernel with a failure