Skip to content

[mlir][SparseTensor] Fix insertion point in createQuickSort #74549

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.

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 1 commit into from
Dec 6, 2023

Conversation

matthias-springer
Copy link
Member

createQuickSort used to generate invalid IR:

"func.func"() <{function_type = (index, index, memref<?xindex>, memref<?xf32>, memref<?xi32>) -> (), sym_name = "_sparse_qsort_0_1_index_coo_1_f32_i32", sym_visibility = "private"}> ({
^bb0(%arg0: index, %arg1: index, %arg2: memref<?xindex>, %arg3: memref<?xf32>, %arg4: memref<?xi32>):
  %0:2 = "scf.while"(%arg0, %arg1) ({
  ^bb0(%arg5: index, %arg6: index):
    // ...
    "scf.condition"(%3, %arg5, %arg6) : (i1, index, index) -> ()
  }, {
  ^bb0(%arg5: index, %arg6: index):
    // ...
    %7:2 = "scf.if"(%6) ({
      %8 = "arith.cmpi"(%2, %3) <{predicate = 7 : i64}> : (index, index) -> i1
      // ...
      "scf.yield"(%9#0, %9#1) : (index, index) -> ()
      %10 = "arith.constant"() <{value = 0 : index}> : () -> index
    }, {
      "scf.yield"(%arg5, %arg5) : (index, index) -> ()
    }) : (i1) -> (index, index)
    "scf.yield"(%7#0, %7#1) : (index, index) -> ()
  }) : (index, index) -> (index, index)
  "func.return"() : () -> ()
}) : () -> ()
within split at mlir/test/Dialect/SparseTensor/buffer_rewriting.mlir:76 offset :11:1: error: 'scf.yield' op must be the last operation in the parent block

This commit fixes tests such as mlir/test/Dialect/SparseTensor/buffer_rewriting.mlir when verifying the IR after each pattern application (#74270).

`createQuickSort` used to generate invalid IR:
```
"func.func"() <{function_type = (index, index, memref<?xindex>, memref<?xf32>, memref<?xi32>) -> (), sym_name = "_sparse_qsort_0_1_index_coo_1_f32_i32", sym_visibility = "private"}> ({
^bb0(%arg0: index, %arg1: index, %arg2: memref<?xindex>, %arg3: memref<?xf32>, %arg4: memref<?xi32>):
  %0:2 = "scf.while"(%arg0, %arg1) ({
  ^bb0(%arg5: index, %arg6: index):
    %1 = "arith.constant"() <{value = 1 : index}> : () -> index
    %2 = "arith.addi"(%arg5, %1) : (index, index) -> index
    %3 = "arith.cmpi"(%2, %arg6) <{predicate = 6 : i64}> : (index, index) -> i1
    "scf.condition"(%3, %arg5, %arg6) : (i1, index, index) -> ()
  }, {
  ^bb0(%arg5: index, %arg6: index):
    %1 = "func.call"(%arg5, %arg6, %arg2, %arg3, %arg4) <{callee = @_sparse_partition_0_1_index_coo_1_f32_i32}> : (index, index, memref<?xindex>, memref<?xf32>, memref<?xi32>) -> index
    %2 = "arith.subi"(%1, %arg5) : (index, index) -> index
    %3 = "arith.subi"(%arg6, %1) : (index, index) -> index
    %4 = "arith.constant"() <{value = 2 : index}> : () -> index
    %5 = "arith.subi"(%arg6, %arg5) : (index, index) -> index
    %6 = "arith.cmpi"(%5, %4) <{predicate = 8 : i64}> : (index, index) -> i1
    %7:2 = "scf.if"(%6) ({
      %8 = "arith.cmpi"(%2, %3) <{predicate = 7 : i64}> : (index, index) -> i1
      // ...
      "scf.yield"(%9#0, %9#1) : (index, index) -> ()
      %10 = "arith.constant"() <{value = 0 : index}> : () -> index
    }, {
      "scf.yield"(%arg5, %arg5) : (index, index) -> ()
    }) : (i1) -> (index, index)
    "scf.yield"(%7#0, %7#1) : (index, index) -> ()
  }) : (index, index) -> (index, index)
  "func.return"() : () -> ()
}) : () -> ()
within split at mlir/test/Dialect/SparseTensor/buffer_rewriting.mlir:76 offset :11:1: error: 'scf.yield' op must be the last operation in the parent block
```

This commit fixes tests such as `mlir/test/Dialect/SparseTensor/buffer_rewriting.mlir` when verifying the IR after each pattern application (llvm#74270).
@llvmbot llvmbot added mlir:sparse Sparse compiler in MLIR mlir labels Dec 6, 2023
@llvmbot
Copy link
Member

llvmbot commented Dec 6, 2023

@llvm/pr-subscribers-mlir-sparse

@llvm/pr-subscribers-mlir

Author: Matthias Springer (matthias-springer)

Changes

createQuickSort used to generate invalid IR:

"func.func"() &lt;{function_type = (index, index, memref&lt;?xindex&gt;, memref&lt;?xf32&gt;, memref&lt;?xi32&gt;) -&gt; (), sym_name = "_sparse_qsort_0_1_index_coo_1_f32_i32", sym_visibility = "private"}&gt; ({
^bb0(%arg0: index, %arg1: index, %arg2: memref&lt;?xindex&gt;, %arg3: memref&lt;?xf32&gt;, %arg4: memref&lt;?xi32&gt;):
  %0:2 = "scf.while"(%arg0, %arg1) ({
  ^bb0(%arg5: index, %arg6: index):
    // ...
    "scf.condition"(%3, %arg5, %arg6) : (i1, index, index) -&gt; ()
  }, {
  ^bb0(%arg5: index, %arg6: index):
    // ...
    %7:2 = "scf.if"(%6) ({
      %8 = "arith.cmpi"(%2, %3) &lt;{predicate = 7 : i64}&gt; : (index, index) -&gt; i1
      // ...
      "scf.yield"(%9#<!-- -->0, %9#<!-- -->1) : (index, index) -&gt; ()
      %10 = "arith.constant"() &lt;{value = 0 : index}&gt; : () -&gt; index
    }, {
      "scf.yield"(%arg5, %arg5) : (index, index) -&gt; ()
    }) : (i1) -&gt; (index, index)
    "scf.yield"(%7#<!-- -->0, %7#<!-- -->1) : (index, index) -&gt; ()
  }) : (index, index) -&gt; (index, index)
  "func.return"() : () -&gt; ()
}) : () -&gt; ()
within split at mlir/test/Dialect/SparseTensor/buffer_rewriting.mlir:76 offset :11:1: error: 'scf.yield' op must be the last operation in the parent block

This commit fixes tests such as mlir/test/Dialect/SparseTensor/buffer_rewriting.mlir when verifying the IR after each pattern application (#74270).


Full diff: https://github.com/llvm/llvm-project/pull/74549.diff

1 Files Affected:

  • (modified) mlir/lib/Dialect/SparseTensor/Transforms/SparseBufferRewriting.cpp (+1-1)
diff --git a/mlir/lib/Dialect/SparseTensor/Transforms/SparseBufferRewriting.cpp b/mlir/lib/Dialect/SparseTensor/Transforms/SparseBufferRewriting.cpp
index 463a49f52283a..cdbf4f048a00f 100644
--- a/mlir/lib/Dialect/SparseTensor/Transforms/SparseBufferRewriting.cpp
+++ b/mlir/lib/Dialect/SparseTensor/Transforms/SparseBufferRewriting.cpp
@@ -952,9 +952,9 @@ createQuickSort(OpBuilder &builder, ModuleOp module, func::FuncOp func,
   Value cond = builder.create<arith::CmpIOp>(loc, arith::CmpIPredicate::ule,
                                              lenLow, lenHigh);
 
+  Value c0 = constantIndex(builder, loc, 0);
   scf::IfOp ifOp = builder.create<scf::IfOp>(loc, types, cond, /*else=*/true);
 
-  Value c0 = constantIndex(builder, loc, 0);
   auto mayRecursion = [&](Value low, Value high, Value len) {
     Value cond =
         builder.create<arith::CmpIOp>(loc, arith::CmpIPredicate::ne, len, c0);

@matthias-springer matthias-springer merged commit 851f85f into llvm:main Dec 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
mlir:sparse Sparse compiler in MLIR mlir
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants