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

[aievec] Canonicalize transposed B contract op. #1475

Merged

Conversation

jsetoain
Copy link
Collaborator

No description provided.

@jsetoain
Copy link
Collaborator Author

@jsetoain jsetoain force-pushed the extract-innermost-dim-perm-from-contract branch 4 times, most recently from 2be4a48 to c2c73de Compare May 21, 2024 15:46
@jsetoain jsetoain marked this pull request as ready for review May 21, 2024 15:48
Comment on lines 54 to 58
SmallVector<vector::IteratorType> iterators = op.getIteratorTypesArray();
auto innerMostIterators =
SmallVector<vector::IteratorType>(iterators.end() - 3, iterators.end());
Copy link
Collaborator

Choose a reason for hiding this comment

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

It's possible that len(iterator_types)<3?

Copy link
Collaborator Author

@jsetoain jsetoain May 21, 2024

Choose a reason for hiding this comment

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

It's possible that len(iterator_types)<3?

I don't think so, no. At that point I've already checked that lhs/rhs/acc have at least 2 dimensions each, which implies at least 2 parallel dimensions (acc) and there has to be at least one contracting dimension.

Edit: I've done the experiment, for the sake of it, and it looks like it does accept having 2d operands and accumulator, and only two iterators (one a reduction). I'm pretty sure that's a mistake, though. I need to check.

@jsetoain jsetoain force-pushed the extract-innermost-dim-perm-from-contract branch from c2c73de to 55e7a06 Compare May 22, 2024 09:59
@jsetoain jsetoain added this pull request to the merge queue May 28, 2024
Merged via the queue into Xilinx:main with commit 6338bc5 May 28, 2024
51 checks passed
@jsetoain jsetoain deleted the extract-innermost-dim-perm-from-contract branch May 28, 2024 13:15
Copy link
Collaborator

@david-vc david-vc left a comment

Choose a reason for hiding this comment

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

Nice work.

@@ -411,6 +460,107 @@ struct FlattenMultDimTransferWritePattern
}
};

// This pattern takes out an implicit transposition of the `rhs` operand in a
Copy link
Collaborator

Choose a reason for hiding this comment

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

I suggest to clarify the comment as follows:
an implicit transposition of the rhs operand
=>
an implicit transposition of the 2 innermost dimensions of the rhs operand

jsetoain added a commit to jsetoain/mlir-aie that referenced this pull request May 29, 2024
singagan pushed a commit that referenced this pull request Jun 5, 2024
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