Skip to content

Conversation

@matthias-springer
Copy link
Member

@matthias-springer matthias-springer commented Apr 28, 2025

This fixes the test when running with ASAN.

@llvmbot
Copy link
Member

llvmbot commented Apr 28, 2025

@llvm/pr-subscribers-mlir-linalg

@llvm/pr-subscribers-mlir

Author: Matthias Springer (matthias-springer)

Changes

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

1 Files Affected:

  • (modified) mlir/test/Integration/Dialect/Linalg/CPU/pack-dynamic-inner-tile.mlir (+6-2)
diff --git a/mlir/test/Integration/Dialect/Linalg/CPU/pack-dynamic-inner-tile.mlir b/mlir/test/Integration/Dialect/Linalg/CPU/pack-dynamic-inner-tile.mlir
index 7b410ccee9633..187d7b862604b 100644
--- a/mlir/test/Integration/Dialect/Linalg/CPU/pack-dynamic-inner-tile.mlir
+++ b/mlir/test/Integration/Dialect/Linalg/CPU/pack-dynamic-inner-tile.mlir
@@ -125,9 +125,13 @@ module @transforms attributes { transform.with_named_sequence } {
     %bufferize = transform.bufferization.one_shot_bufferize %module
       {bufferize_function_boundaries=true} : (!transform.any_op) -> !transform.any_op
 
-    // 5. Canonicalize
+    // 5. Deallocate buffers.
     %func_op_bufferized = transform.structured.match ops{["func.func"]} in %bufferize : (!transform.any_op) -> !transform.op<"func.func">
-    transform.apply_patterns to %func_op_bufferized {
+    %func_op_deallocated = transform.apply_registered_pass "buffer-deallocation-pipeline" to %func_op_bufferized
+      : (!transform.op<"func.func">) -> !transform.op<"func.func">
+
+    // 6. Canonicalize
+    transform.apply_patterns to %func_op_deallocated {
       transform.apply_patterns.canonicalization
     } : !transform.op<"func.func">
 

Copy link
Contributor

@banach-space banach-space left a comment

Choose a reason for hiding this comment

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

Thanks for the fix, LGTM!

This fixes the test when running with ASAN

How do you enable ASAN? LLVM_USE_SANITIZER=memory?

@matthias-springer
Copy link
Member Author

-DLLVM_USE_SANITIZER="Address"

Btw, the unpack test also has a leak, but the buffer deallocation pass inserts the dealloc inside of vector.mask, which is invalid.

@matthias-springer matthias-springer merged commit d1a84b9 into main Apr 28, 2025
14 checks passed
@matthias-springer matthias-springer deleted the users/matthias-springer/fix_leak_28 branch April 28, 2025 15:17
@banach-space
Copy link
Contributor

Btw, the unpack test also has a leak, but the buffer deallocation pass inserts the dealloc inside of vector.mask, which is invalid.

I can look into that if you want, sounds like a familiar issue.

@matthias-springer
Copy link
Member Author

Feel free to take a took. But I think we're going to have to extend the buffer deallocation pass in some way (to make sure that we pick the correct insertion point for the dealloc op)

IanWood1 pushed a commit to IanWood1/llvm-project that referenced this pull request May 6, 2025
GeorgeARM pushed a commit to GeorgeARM/llvm-project that referenced this pull request May 7, 2025
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.

4 participants