Skip to content
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

Fixing dynamic objectFifo #1907

Merged
merged 59 commits into from
Dec 6, 2024
Merged

Fixing dynamic objectFifo #1907

merged 59 commits into from
Dec 6, 2024

Conversation

pvasireddy-amd
Copy link
Collaborator

@pvasireddy-amd pvasireddy-amd commented Nov 5, 2024

Issue #1884

Copy link
Contributor

github-actions bot commented Nov 5, 2024

Coverage Report

Created: 2024-12-05 23:21

Click here for information about interpreting this report.

FilenameFunction CoverageLine CoverageRegion CoverageBranch Coverage
home/runner/work/mlir-aie/mlir-aie/lib/Dialect/AIE/Transforms/AIEObjectFifoStatefulTransform.cpp 100.00% 93.49% 90.11% 84.84%
Totals 100.00% 93.49% 90.11% 84.84%
Generated by llvm-cov -- llvm version 14.0.0

@pvasireddy-amd
Copy link
Collaborator Author

pvasireddy-amd commented Dec 4, 2024

Note: The sliding_window_conditional example is the only one that fails now, but this issue appears unrelated to dynamic objectFifos. The example defines 3 input objectFifos, but only 2 buffers are created when lowered using the --aie-objectFifo-stateful-transform pass. This results in runtime_error. Currently investigating the problem with objectFifo inferring depth in if-else case.

@pvasireddy-amd pvasireddy-amd changed the title [TEST] Adding dynamic objectFifo flag [TEST] Fixing dynamic objectFifo Dec 4, 2024
@pvasireddy-amd
Copy link
Collaborator Author

Since the problem is unrelated to the dynamic objectFifos, I have pushed a working MLIR version of the sliding_window_conditional example into test/npu-xrt/dynamic_object_fifo/sliding_window_conditional. A separate PR will follow with the necessary fixes to enable running the Python version of this example.

@pvasireddy-amd pvasireddy-amd changed the title [TEST] Fixing dynamic objectFifo Fixing dynamic objectFifo Dec 5, 2024
Copy link
Collaborator

@AndraBisca AndraBisca left a comment

Choose a reason for hiding this comment

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

Thank you for the fix!
Regarding the sliding_window_conditional example, it appears that the if-statement does not lower from IRON to MLIR as there is no wrapper similar to the for_ loop statement. As we plan to add this change in a subsequent PR I am happy to merge this fix in already.

@hunhoffe
Copy link
Contributor

hunhoffe commented Dec 6, 2024

For the record, mlir-python-extras should contain some wrappers for if statements e.g., here: https://github.com/makslevental/mlir-python-extras/blob/b19f9b4246a0e092a2f30eaa6dde8584adb64bc5/mlir/extras/dialects/ext/scf.py#L363

I have not attempted to use them in IRON, but wanted to mention them for whoever goes down that path later on!

@pvasireddy-amd pvasireddy-amd added this pull request to the merge queue Dec 6, 2024
Merged via the queue into main with commit ea74c75 Dec 6, 2024
52 checks passed
@pvasireddy-amd pvasireddy-amd deleted the dyn_objFifo_fix branch December 6, 2024 16:19
@pvasireddy-amd pvasireddy-amd linked an issue Dec 6, 2024 that may be closed by this pull request
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.

Dynamic Object-FIFO lowering not functional
5 participants