[Mlir-commits] [mlir] Fix mlir-reduce splitting smallest range instead of the largest one (PR #214738)

[llvmlistbot at llvm.org]

llvmorg-github-actions[bot] wrote:


<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-mlir

@llvm/pr-subscribers-mlir-core

Author: Daria Sukhonina (zetanumbers)

<details>
<summary>Changes</summary>

The function `max_element` expects comparison lambda function to return true if first argument is **less** than the second one, which is the opposite to the current code. As such behavior is different from the one described in the comment above. The difference can become obvious when trying to remove the last line in the `except_one` MLIR function which was newly added to tests. This PR fixes such case.

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


4 Files Affected:

- (modified) mlir/lib/Reducer/ReductionNode.cpp (+1-1) 
- (modified) mlir/test/mlir-reduce/reduction-tree/doc-example.mlir (+5-4) 
- (added) mlir/test/mlir-reduce/reduction-tree/except-last.mlir (+17) 
- (added) mlir/test/mlir-reduce/script/except-last.sh (+7) 


``````````diff
diff --git a/mlir/lib/Reducer/ReductionNode.cpp b/mlir/lib/Reducer/ReductionNode.cpp
index 11aeaf77b4642..f298c12665b83 100644
--- a/mlir/lib/Reducer/ReductionNode.cpp
+++ b/mlir/lib/Reducer/ReductionNode.cpp
@@ -75,7 +75,7 @@ ArrayRef<ReductionNode *> ReductionNode::generateNewVariants() {
   // final ranges vector will be {{1, 3}, {4, 6}, {6, 9}}.
   auto maxElement =
       llvm::max_element(ranges, [](const Range &lhs, const Range &rhs) {
-        return (lhs.second - lhs.first) > (rhs.second - rhs.first);
+        return (lhs.second - lhs.first) < (rhs.second - rhs.first);
       });
 
   // The length of range is less than 1, we can't split it to create new
diff --git a/mlir/test/mlir-reduce/reduction-tree/doc-example.mlir b/mlir/test/mlir-reduce/reduction-tree/doc-example.mlir
index 7f194025a7ef0..0593db2168afe 100644
--- a/mlir/test/mlir-reduce/reduction-tree/doc-example.mlir
+++ b/mlir/test/mlir-reduce/reduction-tree/doc-example.mlir
@@ -26,7 +26,8 @@ func.func @func2(%arg0: i1) -> f32 {
 // CHECK-LABEL: func @func2
 // CHECK-SAME: (%arg0: i1) -> f32
 // CHECK-DAG: %[[C22:.*]] = arith.constant 2.200000e+00 : f32
-// CHECK-DAG: %[[C75:.*]] = arith.constant 7.500000e+00 : f32
-// CHECK: %[[SEL:.*]] = arith.select %arg0, %[[C75]], %[[C22]] : f32
-// CHECK: %[[ADD:.*]] = arith.addf %[[SEL]], %[[C22]] : f32
-// CHECK: return %[[ADD]] : f32
+// CHECK-DAG: %[[C53:.*]] = arith.constant 5.300000e+00 : f32
+// CHECK: %[[ADD1:.*]] = arith.addf %[[C22]], %[[C53]] : f32
+// CHECK: %[[SEL:.*]] = arith.select %arg0, %[[ADD1]], %[[C22]] : f32
+// CHECK: %[[ADD2:.*]] = arith.addf %[[C22]], %[[SEL]] : f32
+// CHECK: return %[[ADD2]] : f32
diff --git a/mlir/test/mlir-reduce/reduction-tree/except-last.mlir b/mlir/test/mlir-reduce/reduction-tree/except-last.mlir
new file mode 100644
index 0000000000000..fa5c59e33177a
--- /dev/null
+++ b/mlir/test/mlir-reduce/reduction-tree/except-last.mlir
@@ -0,0 +1,17 @@
+// UNSUPPORTED: system-windows
+// RUN: mlir-reduce %s -reduction-tree='traversal-mode=0 test=%S/../script/except-last.sh' | FileCheck %s
+// We are testing the ability of keeping trivially-dead yet interesting code
+
+func.func @except_last() {
+  %0 = arith.constant 1 : i32
+  %1 = arith.constant 2 : i32
+  %2 = arith.constant 3 : i32
+  %3 = arith.constant 4 : i32
+  return
+}
+
+// CHECK-LABEL: func @except_last
+//  CHECK-NEXT:   arith.constant 1 : i32
+//  CHECK-NEXT:   arith.constant 2 : i32
+//  CHECK-NEXT:   arith.constant 3 : i32
+//  CHECK-NEXT: return
diff --git a/mlir/test/mlir-reduce/script/except-last.sh b/mlir/test/mlir-reduce/script/except-last.sh
new file mode 100755
index 0000000000000..b6dce52a9403f
--- /dev/null
+++ b/mlir/test/mlir-reduce/script/except-last.sh
@@ -0,0 +1,7 @@
+#!/bin/sh
+
+file=$1
+
+if grep -q "arith.constant 1 : i32" $file && grep -q "arith.constant 2 : i32" $file && grep -q "arith.constant 3 : i32" $file; then
+  exit 1
+fi

``````````

</details>


https://github.com/llvm/llvm-project/pull/214738