[mlir][arith] Add comparison integration tests

[pingshiyu]

Comparison operations regression tests, from the original larger PR that has been broken down: #92272

[pingshiyu]

@banach-space @kuhar

[llvmbot]

@llvm/pr-subscribers-mlir-arith

@llvm/pr-subscribers-mlir

Author: Jacob Yu (pingshiyu)

Changes

Comparison operations regression tests, from the original larger PR that has been broken down: #92272

---

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

1 Files Affected:

• (added) mlir/test/Integration/Dialect/Arith/CPU/comparison.mlir (+61)

diff --git a/mlir/test/Integration/Dialect/Arith/CPU/comparison.mlir b/mlir/test/Integration/Dialect/Arith/CPU/comparison.mlir
new file mode 100644
index 0000000000000..85b70b2cad5a0
--- /dev/null
+++ b/mlir/test/Integration/Dialect/Arith/CPU/comparison.mlir
@@ -0,0 +1,61 @@
+// Tests comparison operations.
+// These tests are intended to be target agnostic: they should yield the same results 
+// regardless of the target platform.
+
+// RUN: mlir-opt %s --convert-scf-to-cf --convert-cf-to-llvm --convert-vector-to-llvm \
+// RUN:             --convert-func-to-llvm --convert-arith-to-llvm | \
+// RUN:   mlir-cpu-runner -e entry -entry-point-result=void \
+// RUN:                   --shared-libs=%mlir_c_runner_utils | \
+// RUN:   FileCheck %s --match-full-lines
+
+func.func @signed_comparison_on_i1s() {
+    // signed comparisons on i1s
+    // slt 0 1 = false, sle 0 1 = false, sgt 0 1 = true, sge 0 1 = true
+    // CHECK:      0
+    // CHECK-NEXT: 0
+    // CHECK-NEXT: 1
+    // CHECK-NEXT: 1
+    %false = arith.constant false
+    %true = arith.constant true
+    %0 = arith.cmpi slt, %false, %true : i1
+    %1 = arith.cmpi sle, %false, %true : i1
+    %2 = arith.cmpi sgt, %false, %true : i1
+    %3 = arith.cmpi sge, %false, %true : i1
+    vector.print %0 : i1
+    vector.print %1 : i1
+    vector.print %2 : i1
+    vector.print %3 : i1
+    return
+}
+
+func.func @sge_0_1_is_true() {
+    // sge 0 -1, sge 0 1, should be true
+    // sge 0 -1 == sge 0 1 == true (1)
+    // CHECK-NEXT: 1
+    // CHECK-NEXT: 1
+    %false = arith.constant 0 : i1
+    %true = arith.constant 1 : i1
+    %true_0 = arith.constant -1 : i1
+    %0 = arith.cmpi sge, %false, %true : i1
+    %1 = arith.cmpi sge, %false, %true_0 : i1
+    vector.print %0 : i1
+    vector.print %1 : i1
+    return
+}
+
+func.func @zero_ult_min_index() {
+    // 0 `ult` -2^63 = true
+    // CHECK-NEXT: 1
+    %c0 = arith.constant 0 : index
+    %c-9223372036854775808 = arith.constant -9223372036854775808 : index
+    %0 = arith.cmpi ult, %c0, %c-9223372036854775808 : index
+    vector.print %0 : i1
+    return
+}
+
+func.func @entry() {
+    func.call @signed_comparison_on_i1s() : () -> ()
+    func.call @sge_0_1_is_true() : () -> ()
+    func.call @zero_ult_min_index() : () -> ()
+    return
+}

[banach-space]

Thanks! Could you try to structure this similarly that what you did for #96973? Similar comment re variable names (prefer underscores to hyphens) and re where to place CHECK lines (near the corresponding vector.print).

It might be easier to finish #96973 and #96975 first, and then just replicate the style that we establish there.

[pingshiyu]

Thanks! Could you try to structure this similarly that what you did for #96973? Similar comment re variable names (prefer underscores to hyphens) and re where to place CHECK lines (near the corresponding vector.print).

It might be easier to finish #96973 and #96975 first, and then just replicate the style that we establish there.

Yep that's the plan! Once the other two PRs are done I'll use the same style as used there (and for the rest of the tests)

[pingshiyu]

I've just pushed a commit updating this with the formats we established in the other PRs.

If everyone is generally happy with the format now and has no further comments, please tick it, and I'll apply the same changes to the other regressions!

[banach-space]

If everyone is generally happy with the format now and has no further comments

I do like the structure (thanks!), but also have a few more comments. I will be sending these shortly. I won't be commenting on other PRs to avoid repetition.

[nit] Please follow https://mlir.llvm.org/getting_started/Contributing/#commit-messages, which links to https://cbea.ms/git-commit/. So, instead of:

[mlir][arith] adding comparison regression tests

it would be:

[mlir][arith] Add comparison regression tests

[pingshiyu]

@banach-space would you be able to take a look at this one too please, and tick if LGTY?

[banach-space]

[nit] "Add comparison regression tests " -> these are usually referred to as either "integration" or "e2e" (end-to-end) tests :)

[banach-space]

Just a few small suggestions

[pingshiyu]

thank you @banach-space

I've pushed a commit to address the comments :)

[banach-space]

A couple more small suggestions. We are getting there 🙏🏻

[pingshiyu]

@banach-space Thanks for the most recent comments!

I've just pushed a commit adding some extra tests and addressing the other comment - please give a tick if all looks good now!

[banach-space]

LGTM, thank you for all the updates and for bearing with me 🙏🏻

[pingshiyu]

Great thank you @banach-space !

One down... 4 more to go 💀

@kuhar would you please be able to have a look as well and if you're happy, tick and merge? (I don't have merge rights)

[kuhar]

LGTM