Skip to content

[mlir][SVE] Add e2e for 1D depthwise WC convolution #85225

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.

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

Merged

Conversation

banach-space
Copy link
Contributor

@banach-space banach-space commented Mar 14, 2024

Follow-up for #81625

@banach-space banach-space force-pushed the andrzej/scalable_depthwise_conv_e2e branch from 02008d4 to f3ec5e7 Compare March 14, 2024 13:40
@banach-space banach-space changed the title andrzej/scalable depthwise conv e2e [mlir][SVE] Add e2e for 1D depthwise WC convolution Mar 14, 2024
Copy link

github-actions bot commented Mar 14, 2024

✅ With the latest revision this PR passed the C/C++ code formatter.

@llvmbot
Copy link
Member

llvmbot commented Mar 16, 2024

@llvm/pr-subscribers-mlir-linalg
@llvm/pr-subscribers-mlir-sve

@llvm/pr-subscribers-mlir

Author: Andrzej Warzyński (banach-space)

Changes

Follow-up for #81625 (please only review the latest commit)


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

1 Files Affected:

  • (added) mlir/test/Integration/Dialect/Linalg/CPU/ArmSVE/1d-depthwise-conv.mlir (+62)
diff --git a/mlir/test/Integration/Dialect/Linalg/CPU/ArmSVE/1d-depthwise-conv.mlir b/mlir/test/Integration/Dialect/Linalg/CPU/ArmSVE/1d-depthwise-conv.mlir
new file mode 100644
index 00000000000000..97c6d1637c0c1d
--- /dev/null
+++ b/mlir/test/Integration/Dialect/Linalg/CPU/ArmSVE/1d-depthwise-conv.mlir
@@ -0,0 +1,62 @@
+// DEFINE: %{compile} =  mlir-opt %s \
+// DEFINE:    -transform-interpreter -test-transform-dialect-erase-schedule \
+// DEFINE:    -one-shot-bufferize="bufferize-function-boundaries" -lower-vector-mask -cse -canonicalize -convert-vector-to-scf -arm-sve-legalize-vector-storage \
+// DEFINE:    -convert-vector-to-llvm="enable-arm-sve" -test-lower-to-llvm -o %t
+// DEFINE: %{entry_point} = conv
+// DEFINE: %{run} = %mcr_aarch64_cmd %t -e %{entry_point} -entry-point-result=void --march=aarch64 --mattr="+sve"\
+// DEFINE:    -shared-libs=%mlir_runner_utils,%mlir_c_runner_utils
+
+// RUN: %{compile} | %{run} | FileCheck %s
+
+func.func @conv() {
+  // Define input/output tensors
+  %input_init = tensor.empty() : tensor<1x8x6xi32>
+  %output_init = tensor.empty() : tensor<1x7x6xi32>
+
+  %five = arith.constant 5 : i32
+  %zero = arith.constant 0 : i32
+  %input = linalg.fill ins(%five : i32) outs(%input_init : tensor<1x8x6xi32>) -> tensor<1x8x6xi32>
+  %output = linalg.fill ins(%zero : i32) outs(%output_init : tensor<1x7x6xi32>) -> tensor<1x7x6xi32>
+
+  // Define the filter tensor
+  %filter = arith.constant dense<[
+    [ 1,  2, 3, 4, 5, 6],
+    [ 11, 12, 13, 14, 15, 16]
+  ]> : tensor<2x6xi32>
+
+  // static sizes -> dynamic sizes
+  %input_dyn = tensor.cast %input_init : tensor<1x8x6xi32> to tensor<1x8x?xi32>
+  %output_dyn = tensor.cast %output : tensor<1x7x6xi32> to tensor<1x7x?xi32>
+  %filter_dyn = tensor.cast %filter : tensor<2x6xi32> to tensor<2x?xi32>
+
+  // Run the convolution
+  %res = linalg.depthwise_conv_1d_nwc_wc
+    {dilations = dense<1> : vector<1xi64>,
+    strides = dense<1> : vector<1xi64>}
+    ins(%input_dyn, %filter_dyn : tensor<1x8x?xi32>, tensor<2x?xi32>)
+    outs(%output_dyn : tensor<1x7x?xi32>) -> tensor<1x7x?xi32>
+
+  // Print the results
+  // CHECK: SVE: START OF TEST OUTPUT
+  vector.print str "SVE: START OF TEST OUTPUT\n"
+
+  // CHECK-NEXT: Unranked Memref base@ = {{.*}} rank = 3 offset = 0 sizes = [1, 7, 6] strides = [42, 6, 1] data =
+  // CHECK-COUNT-7: [60, 70, 80, 90, 100, 110]
+  %xf = tensor.cast %res : tensor<1x7x?xi32> to tensor<*xi32>
+  call @printMemrefI32(%xf) : (tensor<*xi32>) -> ()
+
+  // CHECK-NEXT: SVE: END OF TEST OUTPUT
+  vector.print str "SVE: END OF TEST OUTPUT\n"
+
+  return
+}
+
+module attributes {transform.with_named_sequence} {
+  transform.named_sequence @__transform_main(%arg0: !transform.any_op {transform.readonly}) {
+    %0 = transform.structured.match ops{["linalg.depthwise_conv_1d_nwc_wc"]} in %arg0 : (!transform.any_op) -> !transform.any_op
+    transform.structured.vectorize %0 vector_sizes [1, 7, [8], 2] : !transform.any_op
+    transform.yield
+  }
+}
+
+func.func private @printMemrefI32(%ptr : tensor<*xi32>) attributes { llvm.emit_c_interface }

Copy link
Collaborator

@c-rhodes c-rhodes left a comment

Choose a reason for hiding this comment

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

LGTM cheers

@banach-space banach-space force-pushed the andrzej/scalable_depthwise_conv_e2e branch from ee401a9 to 41de1a7 Compare March 22, 2024 14:07
@banach-space banach-space merged commit 01b1b0c into llvm:main Mar 22, 2024
chencha3 pushed a commit to chencha3/llvm-project that referenced this pull request Mar 23, 2024
@omjavaid
Copy link
Contributor

Breaks following AArch64 SVE buildbots:
https://lab.llvm.org/buildbot/#/builders/184/builds/11363
https://lab.llvm.org/buildbot/#/builders/176/builds/9331

I am temporarily reverting commit 01b1b0c

@banach-space
Copy link
Contributor Author

Breaks following AArch64 SVE buildbots:
https://lab.llvm.org/buildbot/#/builders/184/builds/11363
https://lab.llvm.org/buildbot/#/builders/176/builds/9331

I am temporarily reverting commit 01b1b0c

Thanks for reverting and apologies for the disruption!

@omjavaid Annoyingly, I'm unable to reproduce this failure and the other buildbot that's supposed to run these tests failed for unrelated reasons:

The limited error log suggests miscomputation, but it's impossible to tell what exactly went wrong. Would you be able to run the following and share the output?

/home/tcwg-buildbot/worker/clang-aarch64-sve-vls-2stage/stage2/bin/mlir-opt /home/tcwg-buildbot/worker/clang-aarch64-sve-vls-2stage/llvm/mlir/test/Integration/Dialect/Linalg/CPU/ArmSVE/1d-depthwise-conv.mlir -transform-interpreter -test-transform-dialect-erase-schedule -one-shot-bufferize="bufferize-function-boundaries" -lower-vector-mask -cse -canonicalize -convert-vector-to-scf -arm-sve-legalize-vector-storage -convert-vector-to-llvm="enable-arm-sve" -test-lower-to-llvm

Otherwise I'm rather stuck.

Comment on lines +5 to +9
// DEFINE: %{entry_point} = conv
// DEFINE: %{run} = %mcr_aarch64_cmd %t -e %{entry_point} -entry-point-result=void --march=aarch64 --mattr="+sve"\
// DEFINE: -shared-libs=%mlir_runner_utils,%mlir_c_runner_utils

// RUN: %{compile} | %{run} | FileCheck %s
Copy link
Member

@MacDue MacDue Mar 23, 2024

Choose a reason for hiding this comment

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

Here it's both using input + output files (via -o), but also piping things. This seems to work when %mcr_aarch64_cmd is an emulator, but if you replace it with mlir-cpu-runner (on SVE hardware) the test fails on the first run (but works on the second run).

Copy link
Member

Choose a reason for hiding this comment

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

P.s. -convert-vector-to-scf -arm-sve-legalize-vector-storage -convert-vector-to-llvm="enable-arm-sve" can be replaced with -convert-vector-to-scf=full-unroll.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, good catch!

Yeah, the RUN should be something like this: // RUN: rm -f %t && %{compile} && %{run} | FileCheck %s instead. However, the current RUN line, when fails, produces:

# .---command stderr------------
# | cannot open input file '/llvm-project/build/release_vls/tools/mlir/test/Integration/Dialect/Linalg/CPU/ArmSVE/Output/1d-depthwise-conv.mlir.tmp': No such file or directory
# | could not parse the input IR
# `-----------------------------

rather than (from https://lab.llvm.org/buildbot/#/builders/176/builds/9331):

# .---command stderr------------
# | Error: entry point not found
# `-----------------------------

Copy link
Member

Choose a reason for hiding this comment

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

A guess: It may depend on the shell/system if the result is an empty file or no file (Error: entry point not found seems to indicate the input was empty).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That could be the case 🤔

There's https://github.com/llvm/llvm-project/blob/main/mlir/test/Integration/Dialect/Linalg/CPU/ArmSVE/matmul.mlir that works fine with this configuration - we probably just missed that it failed first time it was run. Also a guess :)

If I don't hear back from Omair with other suggestion, I'll merge the updated version around Monday and just monitor the bots to see what happens. Thanks Ben!

Copy link
Member

Choose a reason for hiding this comment

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

No worries, sounds good to me :)

In the matmul test there's a lone // RUN: %{compile}, with the run steps separate, which would not have this issue.

banach-space added a commit that referenced this pull request Mar 25, 2024
Follow-up for #81625

Relands #85225 with a minor update to the RUN line to fix buildbot
failures:
```diff
-// RUN: %{compile} | %{run} | FileCheck %s
+// RUN: rm -f %t && %{compile} && %{run} | FileCheck %s
```

Failing buildbots after landing #85225:
 * https://lab.llvm.org/buildbot/#/builders/184/builds/11363
 * https://lab.llvm.org/buildbot/#/builders/176/builds/9331
@banach-space
Copy link
Contributor Author

Re-landed: 0bdb883

Buildbot seems fine: https://lab.llvm.org/buildbot/#/builders/184/builds/11408

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants