Skip to content

Conversation

@sunwookim028
Copy link
Owner

## Description ##
This PR adds support for the `memref.copy` operation in the Vivado HLS C++ emitter, resolving a crash that occurred when translating MLIR containing this operation.

### Problems ###
The Vivado HLS C++ emitter previously did not support the `memref.copy` operation, leading to a crash during translation when such operations were present in the MLIR IR. This prevented the generation of correct HLS C++ code for memory copy operations.

### Proposed Solutions ###
1.  **`mlir/include/allo/Dialect/Visitor.h`**: Added `memref::CopyOp` to the `HLSCppVisitorBase` to ensure it is recognized by the visitor.
2.  **`mlir/lib/Translation/EmitVivadoHLS.cpp`**:
    *   Declared and implemented the `emitCopy` function.
    *   The `emitCopy` function translates `memref.copy` operations into nested C++ `for` loops for array element-wise assignment.
    *   It handles the optional `to` attribute to correctly name the target memory region in the generated C++ code.
    *   It includes a check to ensure that only statically shaped memrefs are supported for `memref.copy`, emitting an error for dynamic shapes.

### Examples ###
**MLIR Input:**
```mlir
memref.copy %alloc_1, %alloc {to = "acc"} : memref<4xi32> to memref<4xi32>

Expected HLS C++ Output:

for (int _copy_iv0 = 0; _copy_iv0 < 4; ++_copy_iv0) {
    acc[_copy_iv0] = v1[_copy_iv0];  // Assuming %alloc is named 'acc' and %alloc_1 is 'v1'
}

Checklist

Please make sure to review and check all of these items:

  • PR's title starts with a category (e.g. [Bugfix], [IR], [Builder], etc)
  • All changes have test coverage (It would be good to provide ~2 different test cases to test the robustness of your code)
  • Pass the formatting check locally
  • Code is well-documented

---
<a href="https://cursor.com/background-agent?bcId=bc-f0f21455-47d4-4a5a-9dc8-7d4169119458"><picture><source media="(prefers-color-scheme: dark)" srcset="https://cursor.com/open-in-cursor-dark.svg"><source media="(prefers-color-scheme: light)" srcset="https://cursor.com/open-in-cursor-light.svg"><img alt="Open in Cursor" src="https://cursor.com/open-in-cursor.svg"></picture></a>&nbsp;<a href="https://cursor.com/agents?id=bc-f0f21455-47d4-4a5a-9dc8-7d4169119458"><picture><source media="(prefers-color-scheme: dark)" srcset="https://cursor.com/open-in-web-dark.svg"><source media="(prefers-color-scheme: light)" srcset="https://cursor.com/open-in-web-light.svg"><img alt="Open in Web" src="https://cursor.com/open-in-web.svg"></picture></a>

chhzh123 and others added 30 commits December 3, 2023 17:46
Co-authored-by: Zack Evan Nelson <zen3@zhang-26.ece.cornell.edu>
sunwookim028 and others added 26 commits November 18, 2025 17:28
Co-authored-by: sk3463 <sk3463@cornell.edu>
@cursor
Copy link

cursor bot commented Dec 12, 2025

Cursor Agent can help with this pull request. Just @cursor in comments and I'll start working on changes in this branch.
Learn more about Cursor Agents

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.