[mlir][arith] mul operation regressions

[pingshiyu]

Regressions for the mul operation, a part of the original large PR #92272

[pingshiyu]

@kuhar @banach-space

[llvmbot]

@llvm/pr-subscribers-mlir-arith

@llvm/pr-subscribers-mlir

Author: Jacob Yu (pingshiyu)

Changes

Regressions for the mul operation, a part of the original large PR #92272

---

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

1 Files Affected:

• (added) mlir/test/Integration/Dialect/Arith/CPU/multiplication.mlir (+44)

diff --git a/mlir/test/Integration/Dialect/Arith/CPU/multiplication.mlir b/mlir/test/Integration/Dialect/Arith/CPU/multiplication.mlir
new file mode 100644
index 0000000000000..494bc2af1821e
--- /dev/null
+++ b/mlir/test/Integration/Dialect/Arith/CPU/multiplication.mlir
@@ -0,0 +1,44 @@
+// Tests mul operations and their variants (e.g. extended).
+// 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 @mulsi_extended_on_i1() {
+    // mulsi_extended on i1, tests for overflow bit
+    // mulsi_extended 1, 1 : i1 = (1, 0)
+    // CHECK:      1
+    // CHECK-NEXT: 0
+    %true = arith.constant true
+    %low, %high = arith.mulsi_extended %true, %true : i1
+    vector.print %low : i1
+    vector.print %high : i1
+    return
+}
+
+func.func @mulsi_mului_extended_overflows() {
+    // mulsi and mului extended versions, with overflow
+    // mulsi_extended -100, -100 : i8 = (16, 39); mului_extended -100, -100 : i8 = (16, 95)
+    // CHECK-NEXT:  16
+    // CHECK-NEXT:  39
+    // CHECK-NEXT:  16
+    // CHECK-NEXT:  95
+    %c-100_i8 = arith.constant -100 : i8
+    %low, %high = arith.mulsi_extended %c-100_i8, %c-100_i8 : i8
+    vector.print %low : i8
+    vector.print %high : i8
+    %low_0, %high_1 = arith.mului_extended %c-100_i8, %c-100_i8 : i8
+    vector.print %low_0 : i8
+    vector.print %high_1 : i8
+    return
+}
+
+func.func @entry() {
+    func.call @mulsi_extended_on_i1() : () -> ()
+    func.call @mulsi_mului_extended_overflows() : () -> ()
+    return
+}

[banach-space]

With your latest updates it's so much easier to see what's being tested, thank you! I have one more suggestion re structuring this.

Having @mulsi_extended_i1() and @mulsi_extended_on_i1() makes it a bit hard to understand what the intended difference is (the names are almost identical). How about:

1. Keep @mulsi_extended_i1() and @mulsi_extended_i8()
2. Rename @mulsi_extended_on_i1() as @mulsi_extended() and use it to test all variants of @mulsi_extended_i{n}() that you have? (i.e. @mulsi_extended_i1() and @mulsi_extended_i8())

So, it would look like this (feel free to re-use):

func.func @mulsi_extended_i1(%v1 : i1, %v2 : i1) -> (i1, i1) {
  // Implementation
}

func.func @mulsi_extended_i8(%v1 : i8, %v2 : i8) -> (i8, i8) {
  // Implementation
}

func.func @mulsi_extended() {
  // ------------------------------------------------
  // Test i1
  // ------------------------------------------------
  // mulsi_extended on i1, tests for overflow bit
  // mulsi_extended 1, 1 : i1 = (1, 0)
  %true = arith.constant true
  %false = arith.constant false

  // CHECK:      1
  // CHECK-NEXT: 0
  func.call @mulsi_extended_i1(%true, %true) : (i1, i1) -> (i1, i1)

  // CHECK-NEXT: 0
  // CHECK-NEXT: 0
  func.call @mulsi_extended_i1(%true, %false) : (i1, i1) -> (i1, i1)

  // CHECK-NEXT: 0
  // CHECK-NEXT: 0
  func.call @mulsi_extended_i1(%false, %true) : (i1, i1) -> (i1, i1)

  // CHECK-NEXT: 0
  // CHECK-NEXT: 0
  func.call @mulsi_extended_i1(%false, %false) : (i1, i1) -> (i1, i1)

  // ------------------------------------------------
  // Test i8
  // ------------------------------------------------
  // Some representative tests for i8, e.g. 63 * 2 vs 63 * 3, 64 * 2 vs 64 * (-2), 0 * 127, 1 * (-128)
  
  // ------------------------------------------------
  // Test i16. i32, i64 - TODO
  // ------------------------------------------------ 
}

[banach-space]

LGTM, thank you for addressing my comments and for pushing on this!

[banach-space]

LGTM, thank you for addressing my comments and for pushing on this!