From beeed3a3782f1f65a2b9164b270d8cfdd21dff18 Mon Sep 17 00:00:00 2001 From: Brian Pickrell Date: Tue, 26 Mar 2024 11:56:33 -0700 Subject: [PATCH 1/8] added simplify_dyn_ops matcher to substitute broadcast_with_dims op, and a test case to simplify_dyn_ops_test --- src/simplify_dyn_ops.cpp | 34 ++++++++++++++++++++++++++++++++ test/simplify_dyn_ops_test.cpp | 36 ++++++++++++++++++++++++++++++++++ 2 files changed, 70 insertions(+) diff --git a/src/simplify_dyn_ops.cpp b/src/simplify_dyn_ops.cpp index 5a3d5bce186..7b2a952986c 100644 --- a/src/simplify_dyn_ops.cpp +++ b/src/simplify_dyn_ops.cpp @@ -33,6 +33,39 @@ namespace migraphx { inline namespace MIGRAPHX_INLINE_NS { +/** + * Convert a Broadcast_with_dims op, which always outputs a dynamic shape, + * to a Multibroadcast op with a static output shape attribute. + * + * This conversion is required to be made since runtime target ops can only output + * static shapes. +*/ +struct find_broadcast_with_dims_static +{ + auto matcher() const + { + return match::name("broadcast_with_dims")(match::nargs(2), + match::arg(0)(match::static_shape()), + match::arg(1)(match::is_constant())); + } + + void apply(module& m, const match::matcher_result& mr) const + { + auto ins = mr.result; + auto inputs = ins->inputs(); + + // read the values of arg(1) to create input to multibroadcast + std::vector sizes_vec(inputs.at(0)->get_shape().ndim()); + + inputs.at(1)->eval().visit( + [&](auto output) { + sizes_vec.assign(output.begin(), output.end()); + }); + + m.replace_instruction(ins, make_op("multibroadcast", {{"out_lens", sizes_vec}}), inputs.at(0) ); + } +}; + /** * Convert a Resize op. with Nearest mode to an implementation using Gather op. * From: resize[scales={...}/sizes={...},](static, constant) @@ -586,6 +619,7 @@ struct simplify_select_module_output_shape void simplify_dyn_ops::apply(module& m) const { match::find_matches(m, + find_broadcast_with_dims_static{}, find_resize_static{}, find_static_dimensions_of{}, find_const_alloc_reshapes{}, diff --git a/test/simplify_dyn_ops_test.cpp b/test/simplify_dyn_ops_test.cpp index caad3cb2771..b1c0385c9bd 100644 --- a/test/simplify_dyn_ops_test.cpp +++ b/test/simplify_dyn_ops_test.cpp @@ -34,6 +34,42 @@ void run_pass(migraphx::module& m) migraphx::run_passes(m, {migraphx::simplify_dyn_ops{}, migraphx::dead_code_elimination{}}); } +TEST_CASE(broadcast_with_dims) +{ + migraphx::module m0; + { + // the X input + migraphx::shape input_x{migraphx::shape::float_type, {3, 1, 1}}; + auto inx = m0.add_parameter("x", input_x); + + // the shape input. Broadcast to this + migraphx::shape dims_s{migraphx::shape::int64_type, {4}}; + std::vector dims = {2, 3, 4, 5}; + auto out_dims = m0.add_literal(migraphx::literal{dims_s, dims}); + + auto r = m0.add_instruction( + migraphx::make_op("broadcast_with_dims" ), + inx, + out_dims + ); + m0.add_return({r}); + } + run_pass(m0); + + migraphx::module m1; + { + migraphx::shape sx{migraphx::shape::float_type, {3, 1, 1}}; + auto inx = m1.add_parameter("x", sx); + + auto r = m1.add_instruction( + migraphx::make_op("multibroadcast",{{"out_lens", {2, 3, 4, 5}}}), inx); + m1.add_return({r}); + + } + EXPECT(m0 == m1); + +} + TEST_CASE(resize) { migraphx::module m0; From 96950d626cf847e7287672be494a0e1c470a0817 Mon Sep 17 00:00:00 2001 From: Brian Pickrell Date: Tue, 26 Mar 2024 12:29:42 -0700 Subject: [PATCH 2/8] tidy changes --- .../migraphx/op/broadcast_with_dims.hpp | 4 ++-- src/simplify_dyn_ops.cpp | 19 +++++++++---------- test/simplify_dyn_ops_test.cpp | 14 ++++---------- 3 files changed, 15 insertions(+), 22 deletions(-) diff --git a/src/include/migraphx/op/broadcast_with_dims.hpp b/src/include/migraphx/op/broadcast_with_dims.hpp index 24f2c901ca1..c2631ab49ed 100644 --- a/src/include/migraphx/op/broadcast_with_dims.hpp +++ b/src/include/migraphx/op/broadcast_with_dims.hpp @@ -58,8 +58,8 @@ struct broadcast_with_dims // output tensor rank greater of input_tensor rank or length of dims vector auto input_tensor_shape = inputs.at(0); auto dims_shape = inputs.at(1); - size_t out_ndim = std::max(input_tensor_shape.ndim(), dims_shape.lens().at(0)); - std::size_t max_int = std::numeric_limits::max(); + size_t out_ndim = std::max(input_tensor_shape.ndim(), dims_shape.lens().at(0)); + std::size_t max_int = std::numeric_limits::max(); std::vector dyn_dims(out_ndim, shape::dynamic_dimension{0, max_int}); return {input_tensor_shape.type(), dyn_dims}; diff --git a/src/simplify_dyn_ops.cpp b/src/simplify_dyn_ops.cpp index 7b2a952986c..51c89ec1a7a 100644 --- a/src/simplify_dyn_ops.cpp +++ b/src/simplify_dyn_ops.cpp @@ -36,33 +36,32 @@ inline namespace MIGRAPHX_INLINE_NS { /** * Convert a Broadcast_with_dims op, which always outputs a dynamic shape, * to a Multibroadcast op with a static output shape attribute. - * + * * This conversion is required to be made since runtime target ops can only output * static shapes. -*/ + */ struct find_broadcast_with_dims_static { auto matcher() const { return match::name("broadcast_with_dims")(match::nargs(2), - match::arg(0)(match::static_shape()), - match::arg(1)(match::is_constant())); + match::arg(0)(match::static_shape()), + match::arg(1)(match::is_constant())); } void apply(module& m, const match::matcher_result& mr) const { - auto ins = mr.result; - auto inputs = ins->inputs(); + auto ins = mr.result; + auto inputs = ins->inputs(); // read the values of arg(1) to create input to multibroadcast std::vector sizes_vec(inputs.at(0)->get_shape().ndim()); inputs.at(1)->eval().visit( - [&](auto output) { - sizes_vec.assign(output.begin(), output.end()); - }); + [&](auto output) { sizes_vec.assign(output.begin(), output.end()); }); - m.replace_instruction(ins, make_op("multibroadcast", {{"out_lens", sizes_vec}}), inputs.at(0) ); + m.replace_instruction( + ins, make_op("multibroadcast", {{"out_lens", sizes_vec}}), inputs.at(0)); } }; diff --git a/test/simplify_dyn_ops_test.cpp b/test/simplify_dyn_ops_test.cpp index b1c0385c9bd..059ffe1bfdb 100644 --- a/test/simplify_dyn_ops_test.cpp +++ b/test/simplify_dyn_ops_test.cpp @@ -40,18 +40,14 @@ TEST_CASE(broadcast_with_dims) { // the X input migraphx::shape input_x{migraphx::shape::float_type, {3, 1, 1}}; - auto inx = m0.add_parameter("x", input_x); + auto inx = m0.add_parameter("x", input_x); // the shape input. Broadcast to this migraphx::shape dims_s{migraphx::shape::int64_type, {4}}; std::vector dims = {2, 3, 4, 5}; - auto out_dims = m0.add_literal(migraphx::literal{dims_s, dims}); + auto out_dims = m0.add_literal(migraphx::literal{dims_s, dims}); - auto r = m0.add_instruction( - migraphx::make_op("broadcast_with_dims" ), - inx, - out_dims - ); + auto r = m0.add_instruction(migraphx::make_op("broadcast_with_dims"), inx, out_dims); m0.add_return({r}); } run_pass(m0); @@ -62,12 +58,10 @@ TEST_CASE(broadcast_with_dims) auto inx = m1.add_parameter("x", sx); auto r = m1.add_instruction( - migraphx::make_op("multibroadcast",{{"out_lens", {2, 3, 4, 5}}}), inx); + migraphx::make_op("multibroadcast", {{"out_lens", {2, 3, 4, 5}}}), inx); m1.add_return({r}); - } EXPECT(m0 == m1); - } TEST_CASE(resize) From ad7998d46022e703fe847fa0d697fe56f972e733 Mon Sep 17 00:00:00 2001 From: Brian Pickrell <95253842+bpickrel@users.noreply.github.com> Date: Fri, 29 Mar 2024 08:17:08 -0700 Subject: [PATCH 3/8] Comments --- src/simplify_dyn_ops.cpp | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/simplify_dyn_ops.cpp b/src/simplify_dyn_ops.cpp index 51c89ec1a7a..5cf77be1d1e 100644 --- a/src/simplify_dyn_ops.cpp +++ b/src/simplify_dyn_ops.cpp @@ -37,8 +37,6 @@ inline namespace MIGRAPHX_INLINE_NS { * Convert a Broadcast_with_dims op, which always outputs a dynamic shape, * to a Multibroadcast op with a static output shape attribute. * - * This conversion is required to be made since runtime target ops can only output - * static shapes. */ struct find_broadcast_with_dims_static { From 165d23effbff5adffec36abe499e06ad7ccea55f Mon Sep 17 00:00:00 2001 From: Brian Pickrell <95253842+bpickrel@users.noreply.github.com> Date: Fri, 29 Mar 2024 08:20:07 -0700 Subject: [PATCH 4/8] cosmetic variable name change --- test/simplify_dyn_ops_test.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/simplify_dyn_ops_test.cpp b/test/simplify_dyn_ops_test.cpp index 059ffe1bfdb..0333a144963 100644 --- a/test/simplify_dyn_ops_test.cpp +++ b/test/simplify_dyn_ops_test.cpp @@ -39,8 +39,8 @@ TEST_CASE(broadcast_with_dims) migraphx::module m0; { // the X input - migraphx::shape input_x{migraphx::shape::float_type, {3, 1, 1}}; - auto inx = m0.add_parameter("x", input_x); + migraphx::shape sx{migraphx::shape::float_type, {3, 1, 1}}; + auto inx = m0.add_parameter("x", sx); // the shape input. Broadcast to this migraphx::shape dims_s{migraphx::shape::int64_type, {4}}; From 39b15ce0006b793a86a607533b704409e7a7ceb3 Mon Sep 17 00:00:00 2001 From: Brian Pickrell <95253842+bpickrel@users.noreply.github.com> Date: Fri, 29 Mar 2024 08:27:57 -0700 Subject: [PATCH 5/8] comment Co-authored-by: Charlie Lin --- src/simplify_dyn_ops.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/simplify_dyn_ops.cpp b/src/simplify_dyn_ops.cpp index 5cf77be1d1e..9f78a8706a7 100644 --- a/src/simplify_dyn_ops.cpp +++ b/src/simplify_dyn_ops.cpp @@ -34,8 +34,8 @@ namespace migraphx { inline namespace MIGRAPHX_INLINE_NS { /** - * Convert a Broadcast_with_dims op, which always outputs a dynamic shape, - * to a Multibroadcast op with a static output shape attribute. + * Replace broadcast_with_dims operators with a static input tensor and a constant `dims` input + * into multibroadcast op with a static output shape attribute. * */ struct find_broadcast_with_dims_static From 371a87320b27d7c81361d6baebfabb5016f9d8eb Mon Sep 17 00:00:00 2001 From: Brian Pickrell Date: Mon, 1 Apr 2024 14:09:30 -0700 Subject: [PATCH 6/8] remove duplicated tests --- test/op_shape_test.cpp | 24 ------------------------ 1 file changed, 24 deletions(-) diff --git a/test/op_shape_test.cpp b/test/op_shape_test.cpp index 7698a15ad60..6a0a476c381 100644 --- a/test/op_shape_test.cpp +++ b/test/op_shape_test.cpp @@ -926,30 +926,6 @@ TEST_CASE(broadcast_for_dot_dyn1) s0); } -TEST_CASE(broadcast_with_dims0) -{ - using migraphx::shape; - shape s0{migraphx::shape::float_type, {2, 4}}; - shape s1{migraphx::shape::int64_type, {4}}; - std::size_t max_int = std::numeric_limits::max(); - std::vector dyn_dims(4, shape::dynamic_dimension{0, max_int}); - expect_shape( - shape{shape::float_type, dyn_dims}, migraphx::make_op("broadcast_with_dims"), s0, s1); -} - -TEST_CASE(broadcast_with_dims1) -{ - using migraphx::shape; - shape s0{migraphx::shape::int32_type, {1, 2, 4}}; - shape s1{migraphx::shape::int64_type, {1}}; - std::size_t max_int = std::numeric_limits::max(); - std::vector dyn_dims(3, shape::dynamic_dimension{0, max_int}); - expect_shape(shape{migraphx::shape::int32_type, dyn_dims}, - migraphx::make_op("broadcast_with_dims"), - s0, - s1); -} - TEST_CASE(broadcast_for_dot_dyn2) { migraphx::shape s0{migraphx::shape::float_type, {{6, 12}, {4, 4}, {8, 8}}}; From 21f42b7067d3e74b2f1718ba937cbc7ff026feef Mon Sep 17 00:00:00 2001 From: Brian Pickrell Date: Mon, 8 Apr 2024 10:08:29 -0700 Subject: [PATCH 7/8] added a negative simplify_dyn_ops test --- test/simplify_dyn_ops_test.cpp | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/test/simplify_dyn_ops_test.cpp b/test/simplify_dyn_ops_test.cpp index 0333a144963..ebc79b0bce4 100644 --- a/test/simplify_dyn_ops_test.cpp +++ b/test/simplify_dyn_ops_test.cpp @@ -64,6 +64,26 @@ TEST_CASE(broadcast_with_dims) EXPECT(m0 == m1); } +TEST_CASE(broadcast_with_dims_invalid) +{ + migraphx::module m0; + { + // X input shape is not broadcastable to given shape + migraphx::shape sx{migraphx::shape::float_type, {3, 1, 2}}; + auto inx = m0.add_parameter("x", sx); + + // the shape input. Broadcast to this + migraphx::shape dims_s{migraphx::shape::int64_type, {4}}; + std::vector dims = {2, 3, 4, 5}; + auto out_dims = m0.add_literal(migraphx::literal{dims_s, dims}); + + auto r = m0.add_instruction(migraphx::make_op("broadcast_with_dims"), inx, out_dims); + m0.add_return({r}); + } + // replacement will be rejected by multibroadcast operation + EXPECT(test::throws([&] { run_pass(m0); })); +} + TEST_CASE(resize) { migraphx::module m0; From e8ffc81c17ebe49cdfc9516d89549398daef7ba3 Mon Sep 17 00:00:00 2001 From: Brian Pickrell Date: Mon, 8 Apr 2024 10:14:32 -0700 Subject: [PATCH 8/8] style; changed vector initialization --- src/simplify_dyn_ops.cpp | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/simplify_dyn_ops.cpp b/src/simplify_dyn_ops.cpp index 9f78a8706a7..3785c19609f 100644 --- a/src/simplify_dyn_ops.cpp +++ b/src/simplify_dyn_ops.cpp @@ -34,7 +34,7 @@ namespace migraphx { inline namespace MIGRAPHX_INLINE_NS { /** - * Replace broadcast_with_dims operators with a static input tensor and a constant `dims` input + * Convert broadcast_with_dims operators with a static input tensor and a constant `dims` input * into multibroadcast op with a static output shape attribute. * */ @@ -53,8 +53,7 @@ struct find_broadcast_with_dims_static auto inputs = ins->inputs(); // read the values of arg(1) to create input to multibroadcast - std::vector sizes_vec(inputs.at(0)->get_shape().ndim()); - + std::vector sizes_vec; inputs.at(1)->eval().visit( [&](auto output) { sizes_vec.assign(output.begin(), output.end()); });