nod-ai / iree-amd-aie

IREE plugin repository for the AMD AIE accelerator
Apache License 2.0
46 stars 23 forks source link

[AMD-AIE] Add support for vectorization in distribute-cores-and-objectfifo #493

Closed Abhishek-Varma closed 5 days ago

Abhishek-Varma commented 1 week ago

-- This commit adds support for vectorized matmul in --iree-amdaie-distribute-cores-and-objectfifos pass.

Co-authored-by: Abhishek Varma abhvarma@amd.com Co-authored-by: Vivian Zhang vivian.zhang@amd.com

Abhishek-Varma commented 1 week ago

I prefer listing files in C++, especially if we own them. Mostly because you do things like comment them out, and find where they're used, more easily. If you can point to a style guide for this project, or example of IREE doing this, I'll approve the change though.

Sure, my preferred way for the changes made in this revision majorly stems out of - adding/renaming/deleting passes involves the extra step of not only renaming the main file but also reflecting that in the CMakeLists.txt and re-arranging it within CMakeLists.txt as well.

The GLOB way is just a nice way to take the onus of adding/renaming/re-arranging/deleting away - we just focus on creating/deleting/renaming the pass and other infra should just adapt without us having to worry about the same (in my opinion). A similar reasoning can be extended for the .mlir lit test files as well.

There's no style guide that I'm aware of (yet) that prefers using either file names directly or the GLOB way.

I didn't understand the use-case of finding where a pass is used based on CMakeLists.txt - could you elaborate a bit more and help understand your point? ( We can discuss about this offline - as I'm deciding to remove the last two commits anyway based on the following point )

require additional invocations of cmake configuration to get the build to refresh correctly

I understood this point well and agree on this as I experienced this previously with GLOB - I see this as a strong reason to remove the last two commits as this would be a heavy price to pay for being lazy (the adding/renaming/re-arranging/deleting aspect :-P)

newling commented 1 week ago

Thank you @Abhishek-Varma

I didn't understand the use-case of finding where a pass is used based on CMakeLists.txt

I just meant finding which library a .cpp file is part of, or which test suite a .mlir file is in. You can just grep a file's name, find out which CMakeLists.txt file it is in