Skip to content

[mlir][arith] Add printing integration tests #98184

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.

Sign up for GitHub

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

[mlir][arith] Add printing integration tests #98184

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a5e7d25
added printing fns in the new format
pingshiyu Jul 9, 2024
77676a3
reindented
pingshiyu Jul 11, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 47 additions & 0 deletions mlir/test/Integration/Dialect/Arith/CPU/print.mlir
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
// 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 @i1() {
// printing i1 values
// print(0 : i1) = '0'; print(1 : i1) = '1'; print(-1 : i1) = '1'
// CHECK: 0
// CHECK-NEXT: 1
// CHECK-NEXT: 1
%false = arith.constant false
%true = arith.constant 1 : i1
Comment on lines +13 to +14
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we use either 0 / 1 or false / true for consistency here?

%true_as_n1 = arith.constant -1 : i1
vector.print %false : i1
vector.print %true : i1
vector.print %true_as_n1 : i1
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From what I can tell, this is not testing anything about arith is it? Seems like this is all about testing "vector.print" in itself, or even just the testing runtime we have.
Can you try to make the file location more relevant?

Can we also consolidate all the testing for the runtime in a single execution? (each new lit test isn't free: it'll fork/exec a compiler process, and then execute the result here).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we also consolidate all the testing for the runtime in a single execution? (each new lit test isn't free: it'll fork/exec a compiler process, and then execute the result here).

I advocated for keeping them in separate files - to make sure the tests are minimal and to make it easier to identify what's being tested. That has made reviewing much easier, but I didn't really think about the runtime cost. Consolidation makes sense, but l'd wait until all PR's from #100121 are complete. Is that OK?

return
}

func.func @index() {
// printing index values
// print(0 : index) = '0'; print(1 : index) = '1'; print(-1 : index) = '2^w - 1'
// index constants are printed as unsigned
// vector.print(arith.constant(x)) ~= toUnsignedRepr x
// CHECK-NEXT: 0
// CHECK-NEXT: 1
// CHECK-NEXT: 18446744073709551615
// CHECK-NEXT: 63
%c0 = arith.constant 0 : index
%c1 = arith.constant 1 : index
%cn1 = arith.constant -1 : index
%c63 = arith.constant 63 : index

vector.print %c0 : index
vector.print %c1 : index
vector.print %cn1 : index
vector.print %c63 : index
return
}
Comment on lines +22 to +41
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we specify the index type in the mlir-opt pipeline above? I'm worried this can be printed as u64 or u32 depending on the host


func.func @entry() {
func.call @i1() : () -> ()
func.call @index() : () -> ()
return
}
Loading