Skip to content

[mlir][linalg][nfc] Delete references to args_in/args_out #111517

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Oct 10, 2024
Merged

[mlir][linalg][nfc] Delete references to args_in/args_out #111517

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
0f700fa
[mlir][linalg][nfc] Delete references to args_in/args_out
banach-space Oct 8, 2024
da7c6ee
fixup! [mlir][linalg][nfc] Delete references to args_in/args_out
banach-space Oct 9, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 0 additions & 4 deletions mlir/docs/BufferDeallocationInternals.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -664,8 +664,6 @@ merged into a single step. Canonicalization removes the clone operation and
func.func @reuseTarget(%arg0: memref<2xf32>, %result: memref<2xf32>){
%temp = memref.alloc() : memref<2xf32>
test.generic {
args_in = 1 : i64,
args_out = 1 : i64,
Copy link
Contributor

@javedabsar1 javedabsar1 Oct 8, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just deleting will still leave IR incorrect. The IR below should be like (Notice the ins , outs and math.exp, and braces ...

    %alloc = memref.alloc() : memref<2xf32>
    linalg.generic 
         {indexing_maps = [#map, #map],
           iterator_types = ["parallel"]} 
         ins(%arg1 : memref<2xf32>) 
         outs(%alloc : memref<2xf32>) {
    ^bb0(%in: f32, %out: f32):
      %1 = math.exp %in : f32
      linalg.yield %1 : f32
    }
``

Copy link
Contributor Author

@banach-space banach-space Oct 9, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for spotting that. This example is broken in other ways too, e.g (the parser rejects this):

%result = bufferization.clone %temp : (memref<2xf32>) -> (memref<2xf32>)

Also, at the top of the file:

**Note:** This pass is deprecated. Please use the ownership-based buffer
deallocation pass instead.

I feel that I should skip this file altogether, it clearly hasn't been maintained for a while now.

Copy link
Contributor

@javedabsar1 javedabsar1 Oct 9, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Even if you don’t fix everything … fixing some parts is already an improvement .

Copy link
Contributor Author

@banach-space banach-space Oct 10, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What are you suggesting? Partial update in this file? TBH, I'd rather skip it.

Copy link
Contributor

@javedabsar1 javedabsar1 Oct 10, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Up to you really.
I meant if args_in args_out are present in files like mlir/include/mlir/Dialect/Bufferization/Transforms/Passes.td which lot of folks read, removing it is still better than 'fix everything or fix nothing' approach.
then

Copy link
Contributor Author

@banach-space banach-space Oct 10, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds like I've addressed your comment then? Asking as according to GitHub you are still "requesting changes" :)

indexing_maps = [#map0, #map0],
iterator_types = ["parallel"]} %arg0, %temp {
^bb0(%gen2_arg0: f32, %gen2_arg1: f32):
Expand All @@ -683,8 +681,6 @@ Will be transformed to:
```mlir
func.func @reuseTarget(%arg0: memref<2xf32>, %result: memref<2xf32>){
test.generic {
args_in = 1 : i64,
args_out = 1 : i64,
indexing_maps = [#map0, #map0],
iterator_types = ["parallel"]} %arg0, %result {
^bb0(%gen2_arg0: f32, %gen2_arg1: f32):
Expand Down
7 changes: 0 additions & 7 deletions mlir/include/mlir/Dialect/Bufferization/Transforms/Passes.td
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,6 @@ def BufferDeallocation : Pass<"buffer-deallocation", "func::FuncOp"> {
^bb2:
%0 = memref.alloc() : memref<2xf32>
linalg.generic {
args_in = 1 : i64,
args_out = 1 : i64,
indexing_maps = [#map0, #map0],
iterator_types = ["parallel"]} %arg1, %0 {
^bb0(%gen1_arg0: f32, %gen1_arg1: f32):
Expand Down Expand Up @@ -63,8 +61,6 @@ def BufferDeallocation : Pass<"buffer-deallocation", "func::FuncOp"> {
^bb2: // pred: ^bb0
%1 = memref.alloc() : memref<2xf32>
linalg.generic {
args_in = 1 : i64,
args_out = 1 : i64,
indexing_maps = [#map0, #map0],
iterator_types = ["parallel"]} %arg1, %1 {
^bb0(%arg3: f32, %arg4: f32):
Expand Down Expand Up @@ -143,8 +139,6 @@ def OwnershipBasedBufferDeallocation : Pass<
^bb2:
%0 = memref.alloc() : memref<2xf32>
linalg.generic {
args_in = 1 : i64,
args_out = 1 : i64,
indexing_maps = [#map0, #map0],
iterator_types = ["parallel"]}
outs(%arg1, %0 : memref<2xf32>, memref<2xf32>) {
Expand Down Expand Up @@ -179,7 +173,6 @@ def OwnershipBasedBufferDeallocation : Pass<
indexing_maps = [#map, #map],
iterator_types = ["parallel"]}
outs(%arg1, %alloc : memref<2xf32>, memref<2xf32>)
attrs = {args_in = 1 : i64, args_out = 1 : i64} {
^bb0(%out: f32, %out_0: f32):
%2 = math.exp %out : f32
linalg.yield %2, %out_0 : f32, f32
Expand Down
4 changes: 0 additions & 4 deletions mlir/lib/Dialect/Linalg/Transforms/DropUnitDims.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -178,8 +178,6 @@ struct MoveInitOperandsToInput : public OpRewritePattern<GenericOp> {
/// ]
///
/// #trait = {
/// args_in = 2,
/// args_out = 1,
/// indexing_maps = #accesses,
/// iterator_types = ["parallel", "parallel"],
/// library_call = "some_external_fn"
Expand Down Expand Up @@ -210,8 +208,6 @@ struct MoveInitOperandsToInput : public OpRewritePattern<GenericOp> {
/// ]
///
/// #trait = {
/// args_in = 2,
/// args_out = 1,
/// indexing_maps = #accesses,
/// iterator_types = ["parallel", "parallel"],
/// library_call = "some_external_fn"
Expand Down
14 changes: 0 additions & 14 deletions mlir/test/Dialect/Linalg/loops.mlir
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -254,8 +254,6 @@ func.func @copy_view(%arg0: memref<?xf32, strided<[1], offset: ?>>, %arg1: memre
affine_map<(i, j, k) -> (i, k, j)>
]
#trait2 = {
args_in = 1,
args_out = 2,
iterator_types = ["parallel", "parallel", "parallel"],
indexing_maps = #accesses,
library_call = "some_external_function_name_2",
Expand Down Expand Up @@ -296,8 +294,6 @@ func.func @generic_region(%arg0: memref<?x?xf32, strided<[?, 1], offset: ?>>, %a
// CHECKPARALLEL: store %[[e]], %{{.*}}[%[[i]], %[[k]], %[[j]]] : memref<?x?x?xf32, strided<[?, ?, 1], offset: ?>>

#trait4 = {
args_in = 1,
args_out = 2,
iterator_types = ["parallel", "parallel", "parallel"],
indexing_maps = #accesses,
library_call = "some_external_function_name_2",
Expand Down Expand Up @@ -366,8 +362,6 @@ func.func @generic_index_region(
]

#trait_broadcast = {
args_in = 1,
args_out = 1,
indexing_maps = #broadcast_access,
iterator_types = ["parallel", "parallel"],
library_call = "some_broadcast_external_fn"
Expand Down Expand Up @@ -466,8 +460,6 @@ func.func @generic_index_op_zero_rank(%arg0: memref<i32>, %arg1: memref<3x4xi32>
]

#trait_reduce_1D = {
args_in = 1,
args_out = 1,
indexing_maps = #reduce_1D_access,
iterator_types = ["reduction"],
library_call = "some_reduce_external_fn"
Expand Down Expand Up @@ -510,8 +502,6 @@ func.func @generic_op_1D_reduce(%arg0: memref<?xf32>, %arg1: memref<f32>)
]

#trait_reduce_init_1D = {
args_in = 2,
args_out = 1,
indexing_maps = #reduce_init_1D_access,
iterator_types = ["reduction"],
library_call = "some_reduce_external_fn"
Expand Down Expand Up @@ -559,8 +549,6 @@ func.func @generic_index_op_1D_reduce(%arg0: memref<?xf32>,
// CHECKPARALLEL: store %[[e]], %[[ARG2]][]

#trait_const_fill = {
args_in = 0,
args_out = 1,
indexing_maps = [affine_map<(i) -> (i)>],
iterator_types = ["parallel"],
library_call = "some_external_fn"
Expand Down Expand Up @@ -591,8 +579,6 @@ func.func @generic_const_init(%arg0: memref<?xf32>) {
affine_map<() -> ()>
]
#scalar_trait = {
args_in = 2,
args_out = 1,
iterator_types = [],
indexing_maps = #scalar_access,
library_call = "some_external_fn"
Expand Down
2 changes: 0 additions & 2 deletions mlir/test/Dialect/Linalg/transform-patterns.mlir
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,8 +118,6 @@ module attributes {transform.with_named_sequence} {
affine_map<(m, n, k) -> (m, n)>
]
#generic_matmul_trait = {
args_in = 2,
args_out = 1,
indexing_maps = #matmul_accesses,
library_call = "linalg_matmul",
iterator_types = ["parallel", "parallel", "reduction"]
Expand Down
8 changes: 0 additions & 8 deletions mlir/test/Dialect/Linalg/vectorization-with-patterns.mlir
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,8 +83,6 @@ module attributes {transform.with_named_sequence} {
// -----

#matmul_trait = {
args_in = 2,
args_out = 1,
indexing_maps = [
affine_map<(m, n, k) -> (m, k)>,
affine_map<(m, n, k) -> (k, n)>,
Expand Down Expand Up @@ -125,8 +123,6 @@ module attributes {transform.with_named_sequence} {
// -----

#matmul_transpose_out_trait = {
args_in = 2,
args_out = 1,
indexing_maps = [
affine_map<(m, n, k) -> (m, k)>,
affine_map<(m, n, k) -> (k, n)>,
Expand Down Expand Up @@ -196,8 +192,6 @@ module attributes {transform.with_named_sequence} {
// -----

#matmul_trait = {
args_in = 2,
args_out = 1,
indexing_maps = [
affine_map<(m, n, k) -> (m, k)>,
affine_map<(m, n, k) -> (k, n)>,
Expand Down Expand Up @@ -528,8 +522,6 @@ func.func @generic_vectorize(%arg0: memref<4x256xf32>,
// CHECK-DAG: %[[C0:.*]] = arith.constant 0 : index
%c1_f32 = arith.constant 1.0 : f32
linalg.generic {
args_in = 0 : i64,
args_out = 10 : i64,
indexing_maps = [
affine_map<(d0, d1) -> (d0, d1)>,
affine_map<(d0, d1) -> (d1)>,
Expand Down
Loading