ROCm / AMDMIGraphX

AMD's graph optimization engine.
https://rocm.docs.amd.com/projects/AMDMIGraphX/en/latest/
MIT License
185 stars 85 forks source link

Incorrect output aliases #1252

Open Slimakanzer opened 2 years ago

Slimakanzer commented 2 years ago

The program fails with an error if the last op is slice.

terminate called after throwing an instance of 'migraphx::version_1::exception' what(): /AMDMIGraphX/src/program.cpp:270: operator(): Incorrect shape {float_type, {1, 63, 672, 672}, {28901376, 451584, 672, 1}} for parameter: main:#output_1

Code example

```cpp #include #include #include #include #include #include #include #include #include #include #include #include #include #include int main() { auto target = migraphx::target(migraphx::gpu::target{}); migraphx::module *network_; migraphx::program program_; network_ = program_.get_main_module(); const size_t W = 672; const size_t H = 672; const size_t K = 64; const size_t C = 3; const size_t R = 3; const size_t S = 3; const migraphx::shape input_shape{migraphx::shape::float_type, {1, C, W, H}}; const migraphx::shape filter_shape{migraphx::shape::float_type, {K, C, R, S}}; const auto input = network_->add_parameter("data", input_shape); std::vector weights(filter_shape.elements(), 1); const auto filter = network_->add_literal(migraphx::literal(filter_shape, weights)); auto out = network_->add_instruction( migraphx::make_op("convolution", {{"padding", {1, 1}}, {"stride", {1, 1}}, {"dilation", {1, 1}}, {"group", 1}}), input, filter); auto slice_out_0 = network_->add_instruction( migraphx::make_op("slice", {{"axes", {1}}, {"starts", {0}}, {"ends", {1}}}), out); auto slice_out_1 = network_->add_instruction( migraphx::make_op("slice", {{"axes", {1}}, {"starts", {1}}, {"ends", {K}}}), out); const auto out0 = network_->add_parameter("main:#output_0", slice_out_0->get_shape()); const auto out1 = network_->add_parameter("main:#output_1", slice_out_1->get_shape()); network_->add_return({slice_out_0, slice_out_1}); program_.compile(target); auto param_shapes = program_.get_parameter_shapes(); for (auto &p : param_shapes) { std::cout << p.first << " " << p.second << '\n'; } auto output_shapes = program_.get_output_shapes(); for (auto &o : output_shapes) { std::cout << o << '\n'; } std::vector input_data; input_data.resize(input_shape.elements()); std::vector out0_data; std::vector out1_data; out0_data.resize(output_shapes[0].elements()); out1_data.resize(output_shapes[1].elements()); migraphx::parameter_map pm; pm["data"] = migraphx::gpu::to_gpu(migraphx::argument(input_shape, input_data.data())); pm["main:#output_0"] = migraphx::gpu::to_gpu( migraphx::argument(output_shapes[0], out0_data.data())); pm["main:#output_1"] = migraphx::gpu::to_gpu( migraphx::argument(output_shapes[1], out1_data.data())); auto output = program_.eval(pm); for (auto &o : output) { std::cout << o.get_shape() << " "; } } ```

Local configuration

Root cause

instruction.cpp:get_output_alias returns the input tensor as the slice op defines the next output alias:

https://github.com/ROCmSoftwarePlatform/AMDMIGraphX/blob/c99be32c013a21c41f6ad31154fc12c03eac1124/src/include/migraphx/op/slice.hpp#L114

And then replace_allocate.cpp:create_output_names creates incorrect output_names with tensor from the previous layer instead of sliced tensor.

Solution

I tried the same test program with default output_alias for slice operation and it works correctly. I suppose that other operations like reshape transpose broadcast also may throw an error.

pfultz2 commented 2 years ago

The output alias is correct for slice at it will alias its input buffer. It doesn't make a copy. We could consider inserting copies when the output buffers dont match the return shapes.

ppanchad-amd commented 1 month ago

Hi @Slimakanzer. Is this ticket still relevant? If not, please close the ticket. Thanks!