From af412bde2213216199c7febf207c13e226e37ce6 Mon Sep 17 00:00:00 2001 From: Conan Jeffrey Truong Date: Wed, 25 Jun 2025 12:53:46 -0700 Subject: [PATCH 1/4] Modified prim ops to not abort on error --- kernels/prim_ops/register_prim_ops.cpp | 27 ++++++++++--------------- kernels/prim_ops/test/prim_ops_test.cpp | 4 +++- 2 files changed, 14 insertions(+), 17 deletions(-) diff --git a/kernels/prim_ops/register_prim_ops.cpp b/kernels/prim_ops/register_prim_ops.cpp index 62aead8978f..b9c9e5f7b26 100644 --- a/kernels/prim_ops/register_prim_ops.cpp +++ b/kernels/prim_ops/register_prim_ops.cpp @@ -22,12 +22,11 @@ namespace function { namespace { -#define __ET_PRIM_OP_ERROR_IMPL(a, b, context) \ - else { \ - ET_CHECK_MSG(false, "%zu, %zu", (size_t)a.tag, (size_t)b.tag); \ +#define __ET_PRIM_OP_ERROR_IMPL(a, b, context) \ + else { \ + ET_KERNEL_CHECK(context, false, InvalidType, /* void */); \ } -// TODO Fail using runtime context #define __NUMBER_ET_PRIM_OP_IMPL(operator, stack, context) \ (void)context; \ EValue& a = *stack[0]; \ @@ -168,8 +167,7 @@ static Kernel prim_ops[] = { } else if (a.isDouble() && b.isInt()) { floor_div_double(a.toDouble(), static_cast(b.toInt()), out); } else { - // TODO Fail using runtime context - ET_CHECK_MSG(false, "%zu, %zu", (size_t)a.tag, (size_t)b.tag); + ET_KERNEL_CHECK(context, false, InvalidType, /* void */); } }), @@ -193,8 +191,7 @@ static Kernel prim_ops[] = { } else if (a.isDouble() && b.isInt()) { out = EValue(a.toDouble() / b.toInt()); } else { - // TODO Fail using runtime context - ET_CHECK_MSG(false, "%zu, %zu", (size_t)a.tag, (size_t)b.tag); + ET_KERNEL_CHECK(context, false, InvalidType, /* void */); } }), @@ -214,8 +211,7 @@ static Kernel prim_ops[] = { // TODO: This should be impossible out = EValue(a.toDouble()); } else { - // TODO Fail using runtime context - ET_CHECK_MSG(false, "%zu", (size_t)a.tag); + ET_KERNEL_CHECK(context, false, InvalidType, /* void */); } }), @@ -265,8 +261,7 @@ static Kernel prim_ops[] = { } else if (a.isDouble()) { out = EValue(-a.toDouble()); } else { - // TODO Fail using runtime context - ET_CHECK_MSG(false, "%zu", (size_t)a.tag); + ET_KERNEL_CHECK(context, false, InvalidType, /* void */); } }), @@ -303,7 +298,7 @@ static Kernel prim_ops[] = { if (a.isInt() && b.isInt()) { out = EValue(a.toInt() % b.toInt()); } else { - ET_CHECK_MSG(false, "%zu, %zu", (size_t)a.tag, (size_t)b.tag); + ET_KERNEL_CHECK(context, false, InvalidType, /* void */); } }), @@ -317,7 +312,7 @@ static Kernel prim_ops[] = { if (a.isDouble()) { out = EValue(static_cast(ceil(a.toDouble()))); } else { - ET_CHECK_MSG(false, "Unsupported DType %zu", (size_t)a.tag); + ET_KERNEL_CHECK(context, false, InvalidType, /* void */); } }), @@ -348,7 +343,7 @@ static Kernel prim_ops[] = { out = EValue(static_cast(res)); } else { - ET_CHECK_MSG(false, "Unsupported DType %zu", (size_t)a.tag); + ET_KERNEL_CHECK(context, false, InvalidType, /* void */); } }), @@ -362,7 +357,7 @@ static Kernel prim_ops[] = { if (a.isDouble()) { out = EValue(static_cast(trunc(a.toDouble()))); } else { - ET_CHECK_MSG(false, "%zu", (size_t)a.tag); + ET_KERNEL_CHECK(context, false, InvalidType, /* void */); } }), diff --git a/kernels/prim_ops/test/prim_ops_test.cpp b/kernels/prim_ops/test/prim_ops_test.cpp index 646d248cf79..4aa16c3c1e5 100644 --- a/kernels/prim_ops/test/prim_ops_test.cpp +++ b/kernels/prim_ops/test/prim_ops_test.cpp @@ -325,7 +325,9 @@ TEST_F(RegisterPrimOpsTest, TestNegScalarWithTensorDies) { } // Try to negate a tensor, which should cause a runtime error. - ET_EXPECT_DEATH(getOpsFn("executorch_prim::neg.Scalar")(context_, stack), ""); + ET_EXPECT_KERNEL_FAILURE( + context_, + getOpsFn("executorch_prim::neg.Scalar")(context_, stack)); } TEST_F(RegisterPrimOpsTest, TestETView) { From 2403b8997764387cd3d67b3511fce9a78153d1d1 Mon Sep 17 00:00:00 2001 From: Conan Jeffrey Truong Date: Wed, 25 Jun 2025 13:22:48 -0700 Subject: [PATCH 2/4] Renamed neg scalar error test --- kernels/prim_ops/test/prim_ops_test.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/kernels/prim_ops/test/prim_ops_test.cpp b/kernels/prim_ops/test/prim_ops_test.cpp index 4aa16c3c1e5..157f271579a 100644 --- a/kernels/prim_ops/test/prim_ops_test.cpp +++ b/kernels/prim_ops/test/prim_ops_test.cpp @@ -308,7 +308,7 @@ TEST_F(RegisterPrimOpsTest, NegScalarReturnsCorrectValue) { EXPECT_EQ(stack[1]->toInt(), -5l); } -TEST_F(RegisterPrimOpsTest, TestNegScalarWithTensorDies) { +TEST_F(RegisterPrimOpsTest, TestNegScalarWithTensorFails) { testing::TensorFactory tf; EValue values[2]; From b36737c103b85b4543f4cd6d4112c67a6c8cbd25 Mon Sep 17 00:00:00 2001 From: Conan Jeffrey Truong Date: Thu, 26 Jun 2025 14:14:44 -0700 Subject: [PATCH 3/4] Lint prim_ops_test.cpp --- kernels/prim_ops/test/prim_ops_test.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/kernels/prim_ops/test/prim_ops_test.cpp b/kernels/prim_ops/test/prim_ops_test.cpp index 157f271579a..f0131cb6a18 100644 --- a/kernels/prim_ops/test/prim_ops_test.cpp +++ b/kernels/prim_ops/test/prim_ops_test.cpp @@ -326,8 +326,7 @@ TEST_F(RegisterPrimOpsTest, TestNegScalarWithTensorFails) { // Try to negate a tensor, which should cause a runtime error. ET_EXPECT_KERNEL_FAILURE( - context_, - getOpsFn("executorch_prim::neg.Scalar")(context_, stack)); + context_, getOpsFn("executorch_prim::neg.Scalar")(context_, stack)); } TEST_F(RegisterPrimOpsTest, TestETView) { From bd272f6bf3883ce86aee951feaa6bbe4f70250e1 Mon Sep 17 00:00:00 2001 From: Conan Jeffrey Truong Date: Thu, 26 Jun 2025 14:32:39 -0700 Subject: [PATCH 4/4] Add error messages to kernel checks --- kernels/prim_ops/register_prim_ops.cpp | 65 +++++++++++++++++++++----- 1 file changed, 54 insertions(+), 11 deletions(-) diff --git a/kernels/prim_ops/register_prim_ops.cpp b/kernels/prim_ops/register_prim_ops.cpp index b9c9e5f7b26..7473d6a1ad0 100644 --- a/kernels/prim_ops/register_prim_ops.cpp +++ b/kernels/prim_ops/register_prim_ops.cpp @@ -22,9 +22,16 @@ namespace function { namespace { -#define __ET_PRIM_OP_ERROR_IMPL(a, b, context) \ - else { \ - ET_KERNEL_CHECK(context, false, InvalidType, /* void */); \ +#define __ET_PRIM_OP_ERROR_IMPL(a, b, context) \ + else { \ + ET_KERNEL_CHECK_MSG( \ + context, \ + false, \ + InvalidType, \ + /* void */, \ + "%zu, %zu", \ + (size_t)a.tag, \ + (size_t)b.tag); \ } #define __NUMBER_ET_PRIM_OP_IMPL(operator, stack, context) \ @@ -167,7 +174,14 @@ static Kernel prim_ops[] = { } else if (a.isDouble() && b.isInt()) { floor_div_double(a.toDouble(), static_cast(b.toInt()), out); } else { - ET_KERNEL_CHECK(context, false, InvalidType, /* void */); + ET_KERNEL_CHECK_MSG( + context, + false, + InvalidType, + /* void */, + "%zu, %zu", + (size_t)a.tag, + (size_t)b.tag); } }), @@ -191,7 +205,14 @@ static Kernel prim_ops[] = { } else if (a.isDouble() && b.isInt()) { out = EValue(a.toDouble() / b.toInt()); } else { - ET_KERNEL_CHECK(context, false, InvalidType, /* void */); + ET_KERNEL_CHECK_MSG( + context, + false, + InvalidType, + /* void */, + "%zu, %zu", + (size_t)a.tag, + (size_t)b.tag); } }), @@ -211,7 +232,8 @@ static Kernel prim_ops[] = { // TODO: This should be impossible out = EValue(a.toDouble()); } else { - ET_KERNEL_CHECK(context, false, InvalidType, /* void */); + ET_KERNEL_CHECK_MSG( + context, false, InvalidType, /* void */, "%zu", (size_t)a.tag); } }), @@ -261,7 +283,8 @@ static Kernel prim_ops[] = { } else if (a.isDouble()) { out = EValue(-a.toDouble()); } else { - ET_KERNEL_CHECK(context, false, InvalidType, /* void */); + ET_KERNEL_CHECK_MSG( + context, false, InvalidType, /* void */, "%zu", (size_t)a.tag); } }), @@ -298,7 +321,14 @@ static Kernel prim_ops[] = { if (a.isInt() && b.isInt()) { out = EValue(a.toInt() % b.toInt()); } else { - ET_KERNEL_CHECK(context, false, InvalidType, /* void */); + ET_KERNEL_CHECK_MSG( + context, + false, + InvalidType, + /* void */, + "%zu, %zu", + (size_t)a.tag, + (size_t)b.tag); } }), @@ -312,7 +342,13 @@ static Kernel prim_ops[] = { if (a.isDouble()) { out = EValue(static_cast(ceil(a.toDouble()))); } else { - ET_KERNEL_CHECK(context, false, InvalidType, /* void */); + ET_KERNEL_CHECK_MSG( + context, + false, + InvalidType, + /* void */, + "Unsupported DType %zu", + (size_t)a.tag); } }), @@ -343,7 +379,13 @@ static Kernel prim_ops[] = { out = EValue(static_cast(res)); } else { - ET_KERNEL_CHECK(context, false, InvalidType, /* void */); + ET_KERNEL_CHECK_MSG( + context, + false, + InvalidType, + /* void */, + "Unsupported DType %zu", + (size_t)a.tag); } }), @@ -357,7 +399,8 @@ static Kernel prim_ops[] = { if (a.isDouble()) { out = EValue(static_cast(trunc(a.toDouble()))); } else { - ET_KERNEL_CHECK(context, false, InvalidType, /* void */); + ET_KERNEL_CHECK_MSG( + context, false, InvalidType, /* void */, "%zu", (size_t)a.tag); } }),