From a0e6284655d525876ea1cb172b41a581c265e43d Mon Sep 17 00:00:00 2001 From: Manuel Candales Date: Wed, 25 Jun 2025 13:08:35 -0700 Subject: [PATCH 1/2] [ET][Portable] Test bad alpha values: op_add - Tests for bad alpha values: add.Tensor & add.Scalar - Fixes alpha handling in portable/optimized op_add kernels - Eliminates usage of ET_SWITCH_SCALAR_OBJ_TYPES in optimized op_add kernel Differential Revision: [D77325770](https://our.internmc.facebook.com/intern/diff/D77325770/) [ghstack-poisoned] --- kernels/optimized/cpu/op_add.cpp | 77 ++++++++++----------- kernels/portable/cpu/op_add.cpp | 12 +++- kernels/test/op_add_test.cpp | 112 +++++++++++++++++++++++++++++++ 3 files changed, 159 insertions(+), 42 deletions(-) diff --git a/kernels/optimized/cpu/op_add.cpp b/kernels/optimized/cpu/op_add.cpp index de16429c598..b933294a249 100644 --- a/kernels/optimized/cpu/op_add.cpp +++ b/kernels/optimized/cpu/op_add.cpp @@ -46,7 +46,7 @@ Tensor& opt_add_out( CTYPE alpha_val; ET_KERNEL_CHECK( ctx, - torch::executor::native::utils::extract_scalar(alpha, &alpha_val), + utils::extract_scalar(alpha, &alpha_val), InvalidArgument, ); CTYPE_B b_val = *b.const_data_ptr(); CTYPE b_casted = static_cast(b_val); @@ -81,7 +81,6 @@ Tensor& opt_add_scalar_out( (void)ctx; ScalarType a_type = a.scalar_type(); - ScalarType b_type = utils::get_scalar_dtype(b); ScalarType common_type = utils::promote_type_with_scalar(a_type, b, /*half_to_float*/ false); ScalarType out_type = out.scalar_type(); @@ -99,47 +98,45 @@ Tensor& opt_add_scalar_out( if (a_type == common_type && a_type == out_type && a_type != ScalarType::Half && a_type != ScalarType::BFloat16) { ET_SWITCH_REALB_TYPES(a_type, ctx, "add.Scalar_out", CTYPE, [&]() { - ET_SWITCH_SCALAR_OBJ_TYPES(b_type, ctx, "add.Scalar_out", CTYPE_B, [&]() { - CTYPE_B b_val; - ET_EXTRACT_SCALAR(b, b_val); - CTYPE b_casted = static_cast(b_val); - CTYPE alpha_val; - ET_EXTRACT_SCALAR(alpha, alpha_val); - - using Vec = at::vec::Vectorized; - at::vec::map( - [alpha_val, b_casted](Vec x) { - return x + Vec(alpha_val * b_casted); - }, - out.mutable_data_ptr(), - a.const_data_ptr(), - out.numel()); - }); + CTYPE b_casted = utils::scalar_to(b); + CTYPE alpha_val; + ET_KERNEL_CHECK( + ctx, + utils::extract_scalar(alpha, &alpha_val), + InvalidArgument, ); + + using Vec = at::vec::Vectorized; + at::vec::map( + [alpha_val, b_casted](Vec x) { + return x + Vec(alpha_val * b_casted); + }, + out.mutable_data_ptr(), + a.const_data_ptr(), + out.numel()); }); } else { ET_SWITCH_REALHBBF16_TYPES(a_type, ctx, "add.Scalar_out", CTYPE_A, [&]() { - ET_SWITCH_SCALAR_OBJ_TYPES(b_type, ctx, "add.Scalar_out", CTYPE_B, [&]() { - ET_SWITCH_REALB_TYPES( - common_type, ctx, "add.Scalar_out", CTYPE_IN, [&]() { - ET_SWITCH_REALHBBF16_TYPES( - out_type, ctx, "add.Scalar_out", CTYPE_OUT, [&]() { - CTYPE_B b_val; - ET_EXTRACT_SCALAR(b, b_val); - CTYPE_IN b_casted = static_cast(b_val); - CTYPE_IN alpha_val; - ET_EXTRACT_SCALAR(alpha, alpha_val); - - const size_t n = a.numel(); - const CTYPE_A* a_data = a.const_data_ptr(); - CTYPE_OUT* out_data = out.mutable_data_ptr(); - for (auto i = 0; i < n; ++i) { - out_data[i] = static_cast( - static_cast(a_data[i]) + - alpha_val * b_casted); - } - }); - }); - }); + ET_SWITCH_REALB_TYPES( + common_type, ctx, "add.Scalar_out", CTYPE_IN, [&]() { + ET_SWITCH_REALHBBF16_TYPES( + out_type, ctx, "add.Scalar_out", CTYPE_OUT, [&]() { + CTYPE_IN b_casted = utils::scalar_to(b); + CTYPE_IN alpha_val; + ET_KERNEL_CHECK( + ctx, + utils::extract_scalar(alpha, &alpha_val), + InvalidArgument, ); + + const size_t n = a.numel(); + const CTYPE_A* a_data = a.const_data_ptr(); + CTYPE_OUT* out_data = out.mutable_data_ptr(); + for (auto i = 0; i < n; ++i) { + out_data[i] = static_cast( + static_cast(a_data[i]) + + alpha_val * b_casted); + } + }); + }); }); } diff --git a/kernels/portable/cpu/op_add.cpp b/kernels/portable/cpu/op_add.cpp index 83642c4864d..4fb1f2d9e88 100644 --- a/kernels/portable/cpu/op_add.cpp +++ b/kernels/portable/cpu/op_add.cpp @@ -51,7 +51,11 @@ Tensor& add_out( static constexpr const char op_name[] = "add.out"; ET_SWITCH_REALB_TYPES(compute_type, ctx, op_name, CTYPE_COMPUTE, [&]() { - const CTYPE_COMPUTE val_alpha = utils::scalar_to(alpha); + CTYPE_COMPUTE val_alpha; + ET_KERNEL_CHECK( + ctx, + utils::extract_scalar(alpha, &val_alpha), + InvalidArgument, ); utils::apply_bitensor_elementwise_fn< CTYPE_COMPUTE, op_name, @@ -103,7 +107,11 @@ Tensor& add_scalar_out( ET_SWITCH_REALB_TYPES(compute_type, ctx, op_name, CTYPE_COMPUTE, [&]() { CTYPE_COMPUTE val_b = utils::scalar_to(b); - CTYPE_COMPUTE val_alpha = utils::scalar_to(alpha); + CTYPE_COMPUTE val_alpha; + ET_KERNEL_CHECK( + ctx, + utils::extract_scalar(alpha, &val_alpha), + InvalidArgument, ); auto val_alpha_times_b = val_alpha * val_b; utils::apply_unitensor_elementwise_fn< CTYPE_COMPUTE, diff --git a/kernels/test/op_add_test.cpp b/kernels/test/op_add_test.cpp index c84341aa9b1..f0e082b43ad 100644 --- a/kernels/test/op_add_test.cpp +++ b/kernels/test/op_add_test.cpp @@ -231,6 +231,17 @@ class OpAddOutKernelTest : public OperatorTest { EXPECT_TENSOR_CLOSE(op_add_out(a, b, 1.0, out), expected); EXPECT_TENSOR_CLOSE(op_add_out(b, a, 1.0, out), expected); } + + template + void expect_bad_alpha_value_dies(Scalar bad_value) { + TensorFactory tf; + Tensor a = tf.ones({2, 2}); + Tensor b = tf.ones({2, 2}); + Tensor out = tf.zeros({2, 2}); + + ET_EXPECT_KERNEL_FAILURE( + context_, op_add_out(a, b, bad_value, out)); + } }; class OpAddScalarOutKernelTest : public OperatorTest { @@ -242,6 +253,17 @@ class OpAddScalarOutKernelTest : public OperatorTest { Tensor& out) { return torch::executor::aten::add_outf(context_, self, other, alpha, out); } + + template + void expect_bad_alpha_value_dies(Scalar bad_value) { + TensorFactory tf; + Tensor a = tf.ones({2, 2}); + Scalar b = 1; + Tensor out = tf.zeros({2, 2}); + + ET_EXPECT_KERNEL_FAILURE( + context_, op_add_scalar_out(a, b, bad_value, out)); + } }; /** @@ -794,3 +816,93 @@ TEST_F(OpAddScalarOutKernelTest, DtypeTest_float16_bool_int_float16) { op_add_scalar_out(self, other, alpha, out); EXPECT_TENSOR_CLOSE(out, out_expected); } + +TEST_F(OpAddOutKernelTest, ByteTensorTooLargeAlphaDies) { + // Cannot be represented by a uint8_t. + expect_bad_alpha_value_dies(256); +} + +TEST_F(OpAddOutKernelTest, ByteTensorFloatingPointAlphaDies) { + // Cannot be represented by a uint8_t. + expect_bad_alpha_value_dies(2.2); +} + +#ifndef USE_ATEN_LIB +TEST_F(OpAddOutKernelTest, IntTensorTooSmallAlphaDies) { + // Cannot be represented by a int32_t. + expect_bad_alpha_value_dies(-2147483649); +} + +TEST_F(OpAddOutKernelTest, IntTensorTooLargeAlphaDies) { + // Cannot be represented by a int32_t. + expect_bad_alpha_value_dies(2147483648); +} +#endif + +TEST_F(OpAddOutKernelTest, IntTensorFloatingPointAlphaDies) { + // Cannot be represented by a uint32_t. + expect_bad_alpha_value_dies(2.2); +} + +TEST_F(OpAddOutKernelTest, FloatTensorTooSmallAlphaDies) { + // Cannot be represented by a float. + expect_bad_alpha_value_dies(-3.41e+38); +} + +TEST_F(OpAddOutKernelTest, FloatTensorTooLargeAlphaDies) { + // Cannot be represented by a float. + expect_bad_alpha_value_dies(3.41e+38); +} + +TEST_F(OpAddOutKernelTest, HalfTensorTooLargeAlphaDies) { + if (!torch::executor::testing::SupportedFeatures::get()->is_aten) { + GTEST_SKIP() << "Portable kernel does the computation in float"; + } + // Cannot be represented by a float. + expect_bad_alpha_value_dies(65505.0); +} + +TEST_F(OpAddScalarOutKernelTest, ByteTensorTooLargeAlphaDies) { + // Cannot be represented by a uint8_t. + expect_bad_alpha_value_dies(256); +} + +TEST_F(OpAddScalarOutKernelTest, ByteTensorFloatingPointAlphaDies) { + // Cannot be represented by a uint8_t. + expect_bad_alpha_value_dies(2.2); +} + +#ifndef USE_ATEN_LIB +TEST_F(OpAddScalarOutKernelTest, IntTensorTooSmallAlphaDies) { + // Cannot be represented by a int32_t. + expect_bad_alpha_value_dies(-2147483649); +} + +TEST_F(OpAddScalarOutKernelTest, IntTensorTooLargeAlphaDies) { + // Cannot be represented by a int32_t. + expect_bad_alpha_value_dies(2147483648); +} +#endif + +TEST_F(OpAddScalarOutKernelTest, IntTensorFloatingPointAlphaDies) { + // Cannot be represented by a uint32_t. + expect_bad_alpha_value_dies(2.2); +} + +TEST_F(OpAddScalarOutKernelTest, FloatTensorTooSmallAlphaDies) { + // Cannot be represented by a float. + expect_bad_alpha_value_dies(-3.41e+38); +} + +TEST_F(OpAddScalarOutKernelTest, FloatTensorTooLargeAlphaDies) { + // Cannot be represented by a float. + expect_bad_alpha_value_dies(3.41e+38); +} + +TEST_F(OpAddScalarOutKernelTest, HalfTensorTooLargeAlphaDies) { + if (!torch::executor::testing::SupportedFeatures::get()->is_aten) { + GTEST_SKIP() << "Portable kernel does the computation in float"; + } + // Cannot be represented by a float. + expect_bad_alpha_value_dies(65505.0); +} From fb5f696effd76741ce45e0e3df81667c722473f5 Mon Sep 17 00:00:00 2001 From: Manuel Candales Date: Wed, 25 Jun 2025 21:31:14 -0700 Subject: [PATCH 2/2] Update on "[ET][Portable] Test bad alpha values: op_add" - Tests for bad alpha values: add.Tensor & add.Scalar - Fixes alpha handling in portable/optimized op_add kernels - Eliminates usage of ET_SWITCH_SCALAR_OBJ_TYPES in optimized op_add kernel Differential Revision: [D77325770](https://our.internmc.facebook.com/intern/diff/D77325770/) [ghstack-poisoned] --- kernels/optimized/cpu/op_add.cpp | 8 ++------ kernels/portable/cpu/op_add.cpp | 8 ++------ kernels/test/op_add_test.cpp | 8 ++------ 3 files changed, 6 insertions(+), 18 deletions(-) diff --git a/kernels/optimized/cpu/op_add.cpp b/kernels/optimized/cpu/op_add.cpp index b933294a249..97bdb0a0d5e 100644 --- a/kernels/optimized/cpu/op_add.cpp +++ b/kernels/optimized/cpu/op_add.cpp @@ -45,9 +45,7 @@ Tensor& opt_add_out( ET_SWITCH_REALB_TYPES(b_type, ctx, "add.out", CTYPE_B, [&]() { CTYPE alpha_val; ET_KERNEL_CHECK( - ctx, - utils::extract_scalar(alpha, &alpha_val), - InvalidArgument, ); + ctx, utils::extract_scalar(alpha, &alpha_val), InvalidArgument, ); CTYPE_B b_val = *b.const_data_ptr(); CTYPE b_casted = static_cast(b_val); @@ -101,9 +99,7 @@ Tensor& opt_add_scalar_out( CTYPE b_casted = utils::scalar_to(b); CTYPE alpha_val; ET_KERNEL_CHECK( - ctx, - utils::extract_scalar(alpha, &alpha_val), - InvalidArgument, ); + ctx, utils::extract_scalar(alpha, &alpha_val), InvalidArgument, ); using Vec = at::vec::Vectorized; at::vec::map( diff --git a/kernels/portable/cpu/op_add.cpp b/kernels/portable/cpu/op_add.cpp index 4fb1f2d9e88..e10534cd233 100644 --- a/kernels/portable/cpu/op_add.cpp +++ b/kernels/portable/cpu/op_add.cpp @@ -53,9 +53,7 @@ Tensor& add_out( ET_SWITCH_REALB_TYPES(compute_type, ctx, op_name, CTYPE_COMPUTE, [&]() { CTYPE_COMPUTE val_alpha; ET_KERNEL_CHECK( - ctx, - utils::extract_scalar(alpha, &val_alpha), - InvalidArgument, ); + ctx, utils::extract_scalar(alpha, &val_alpha), InvalidArgument, ); utils::apply_bitensor_elementwise_fn< CTYPE_COMPUTE, op_name, @@ -109,9 +107,7 @@ Tensor& add_scalar_out( CTYPE_COMPUTE val_b = utils::scalar_to(b); CTYPE_COMPUTE val_alpha; ET_KERNEL_CHECK( - ctx, - utils::extract_scalar(alpha, &val_alpha), - InvalidArgument, ); + ctx, utils::extract_scalar(alpha, &val_alpha), InvalidArgument, ); auto val_alpha_times_b = val_alpha * val_b; utils::apply_unitensor_elementwise_fn< CTYPE_COMPUTE, diff --git a/kernels/test/op_add_test.cpp b/kernels/test/op_add_test.cpp index f0e082b43ad..8fa51c49dfa 100644 --- a/kernels/test/op_add_test.cpp +++ b/kernels/test/op_add_test.cpp @@ -15,8 +15,6 @@ #include -#include - using namespace ::testing; using executorch::aten::Scalar; using executorch::aten::ScalarType; @@ -239,8 +237,7 @@ class OpAddOutKernelTest : public OperatorTest { Tensor b = tf.ones({2, 2}); Tensor out = tf.zeros({2, 2}); - ET_EXPECT_KERNEL_FAILURE( - context_, op_add_out(a, b, bad_value, out)); + ET_EXPECT_KERNEL_FAILURE(context_, op_add_out(a, b, bad_value, out)); } }; @@ -261,8 +258,7 @@ class OpAddScalarOutKernelTest : public OperatorTest { Scalar b = 1; Tensor out = tf.zeros({2, 2}); - ET_EXPECT_KERNEL_FAILURE( - context_, op_add_scalar_out(a, b, bad_value, out)); + ET_EXPECT_KERNEL_FAILURE(context_, op_add_scalar_out(a, b, bad_value, out)); } };