Skip to content

Conversation

@hunhoffe
Copy link
Collaborator

@hunhoffe hunhoffe commented Sep 12, 2024

I had introduced a bug in both matrix multiply whole_array and matrix multiply cascade. In a previous PR, I fixed it for whole_array but did not realize the bug affected cascade as well until it occurred to me to double check recently.

@hunhoffe
Copy link
Collaborator Author

hunhoffe commented Sep 12, 2024

I believe the change I committed fixes the bug, but am running the whole matrix multiply sweep.sh on cascade locally to be more sure before I set this PR as ready for review. This may be related to #1751 but I did not verify (although I do know the bug can present with that type of error). This bug would not have affected matrix_vector, as matrix_vector does not contain any object_fifo_links with offsets, so at most it partially addresses the issue.

At the least, the following case failed before this commit but succeeds after this commit:

M?=640
K?=896
N?=768
m?=16
k?=32
n?=48

@hunhoffe
Copy link
Collaborator Author

hunhoffe commented Sep 13, 2024

I tried config:

M?=32
K?=512
N?=512
m?=32
k?=64
n?=32

and it passed. This may address (one of) the problems raised in the (possibly related) issue (namely, M = 32, n = 32).

@hunhoffe hunhoffe marked this pull request as ready for review September 13, 2024 17:12
@hunhoffe hunhoffe added this pull request to the merge queue Sep 16, 2024
Merged via the queue into main with commit 932a5df Sep 16, 2024
@hunhoffe hunhoffe deleted the matmul-cascade-link-offsets branch September 16, 2024 17:22
fifield pushed a commit to fifield/mlir-aie that referenced this pull request Nov 12, 2025
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.

3 participants