[Mlir-commits] [mlir] [mlir][arith] Add comparison regression tests (PR #96974)

[Andrzej Warzyński]

================
@@ -0,0 +1,115 @@
+// 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 @cmpi_slt_i1(%v1 : i1, %v2 : i1) {
+  vector.print str "@cmpi_slt_i1\n"
+  %res = arith.cmpi slt, %v1, %v2 : i1
+  vector.print %res : i1
+  return
+}
+
+func.func @cmpi_sle_i1(%v1 : i1, %v2 : i1) {
+  vector.print str "@cmpi_sle_i1\n"
+  %res = arith.cmpi sle, %v1, %v2 : i1
+  vector.print %res : i1
+  return
+}
+
+func.func @cmpi_sgt_i1(%v1 : i1, %v2 : i1) {
+  vector.print str "@cmpi_sgt_i1\n"
+  %res = arith.cmpi sgt, %v1, %v2 : i1
+  vector.print %res : i1
+  return
+}
+
+func.func @cmpi_sge_i1(%v1 : i1, %v2 : i1) {
+  vector.print str "@cmpi_sge_i1\n"
+  %res = arith.cmpi sge, %v1, %v2 : i1
+  vector.print %res : i1
+  return
+}
+
+func.func @cmpi_signed() {
+  // ------------------------------------------------
+  // Test i1
+  // ------------------------------------------------
+  %false_i1 = arith.constant 0 : i1
+  %true_i1 = arith.constant 1 : i1
+  %true_i1_n1 = arith.constant -1 : i1
+
+  // sge 0 -1, sge 0 1, should be true
+  // since the bitvector `1` is interpreted as the int value -1 in signed comparisons
+  // sge 0 -1 == sge 0 1 == true (1)
----------------
banach-space wrote:

This is effectively duplicating L43. Also, IMHO can be skipped. 

In general, I'd only keep your comment from L44 (and remove L43 and L45). The main "tricky" part here is the fact that `1` is interpreted as "-1" in signed comparison. That definitely merits a comment. 

Also, it looks like it most test in the block for `i1` test this behaviour. I'd consider this:
```mlir
  // -----------------------------------------------------------------------------
  // Test i1
  // Note  - bitvector `1` is interpreted as the int value -1 in signed comparison
  // -----------------------------------------------------------------------------
```

Feel free to re-use. Trim the comment to 80 cols and use consistent formatting ;-) 

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