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.

Sign up for GitHub

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

Jump to bottom

[Vectorization][ObjectFifo] Enable larger Matmul + Truncf #856

Merged
merged 17 commits into from
Nov 5, 2024
Merged

[Vectorization][ObjectFifo] Enable larger Matmul + Truncf #856

merged 17 commits into from
Nov 5, 2024

Conversation

Abhishek-Varma
Copy link
Contributor

@Abhishek-Varma Abhishek-Varma commented Oct 23, 2024 •
edited
Loading

-- This PR adds the following pieces to enable working of larger Matmul + Truncf.

  1. Updates insert-loops-for-vectorization to work on bufferized inputs, collapse unit dimensions and coalesce the generated loops for tiling.
  2. Makes the pipeline as IREEComprehensiveBufferization -> insert-cores -> insert-loops-for-vectorization -> vectorization.
  3. Creates and adds a new pass function-outlining - which is invoked right after insert-cores pass.
  4. Adds two canonicalization patterns that canonicalizes away trivial read/write access patterns to enable other dependent canonicalizations in aievec.
  5. Adds e2e CI test for 128x128x256 Matmul + Truncf bf16 -> bf16.

Signed-off-by: Abhishek Varma abhvarma@amd.com

@Abhishek-Varma Abhishek-Varma changed the base branch from main to avarma_test_buffer_then_vectorize October 23, 2024 13:02
@Abhishek-Varma Abhishek-Varma force-pushed the avarma_test_buffer_then_vectorize branch from 50f009b to 4f6698f Compare October 28, 2024 05:31
@Abhishek-Varma Abhishek-Varma changed the base branch from avarma_test_buffer_then_vectorize to main October 28, 2024 07:35
@Abhishek-Varma Abhishek-Varma changed the title Add function outlining pass + e2e larger Matmul + Truncf [Vectorization][ObjectFifo] Enable larger Matmul + Truncf Oct 28, 2024
@Abhishek-Varma Abhishek-Varma marked this pull request as ready for review October 28, 2024 10:58
This was referenced Oct 28, 2024
Closed
Closed
jtuyls
jtuyls requested changes Oct 28, 2024
View reviewed changes
build_tools/ci/cpu_comparison/run.py Outdated Show resolved Hide resolved
build_tools/ci/cpu_comparison/run.py Outdated Show resolved Hide resolved
compiler/plugins/target/AMD-AIE/aievec/VectorToVectorConversions.cpp Show resolved Hide resolved
compiler/plugins/target/AMD-AIE/aievec/VectorToVectorConversions.cpp Show resolved Hide resolved
compiler/plugins/target/AMD-AIE/aievec/VectorToVectorConversions.cpp Outdated Show resolved Hide resolved
compiler/plugins/target/AMD-AIE/iree-amd-aie/Transforms/AMDAIEInsertLoopsForVectorization.cpp Outdated Show resolved Hide resolved
compiler/plugins/target/AMD-AIE/iree-amd-aie/Transforms/AMDAIEInsertLoopsForVectorization.cpp Outdated Show resolved Hide resolved
compiler/plugins/target/AMD-AIE/iree-amd-aie/Transforms/AMDAIEInsertLoopsForVectorization.cpp Outdated Show resolved Hide resolved
compiler/plugins/target/AMD-AIE/iree-amd-aie/Transforms/Passes.td Outdated Show resolved Hide resolved
...iler/plugins/target/AMD-AIE/iree-amd-aie/Transforms/test/insert_loops_for_vectorization.mlir Show resolved Hide resolved
newling
newling requested changes Oct 28, 2024
View reviewed changes
Copy link
Contributor

@newling newling left a comment

Choose a reason for hiding this comment

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

I think this should be at divided into multiple PRs, one containing just the function outlining.

Nice to see this approach working!

compiler/plugins/target/AMD-AIE/iree-amd-aie/Transforms/test/function_outlining.mlir Outdated Show resolved Hide resolved
yzhang93
yzhang93 requested changes Oct 28, 2024
View reviewed changes
Copy link
Contributor

@yzhang93 yzhang93 left a comment

Choose a reason for hiding this comment

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

Nice progress! This PR needs a bit more polish. Also please talk with @erwei-xilinx and see if he would help to make AIR tests work with the changes.

build_tools/ci/run_matmul_test.sh Outdated Show resolved Hide resolved
build_tools/ci/cpu_comparison/run.py Outdated Show resolved Hide resolved
compiler/plugins/target/AMD-AIE/aievec/test/precanonicalization-aieml-llvmir.mlir Show resolved Hide resolved
compiler/plugins/target/AMD-AIE/iree-amd-aie/Transforms/AMDAIEFunctionOutlining.cpp Outdated Show resolved Hide resolved
compiler/plugins/target/AMD-AIE/iree-amd-aie/Transforms/AMDAIEFunctionOutlining.cpp Outdated Show resolved Hide resolved
compiler/plugins/target/AMD-AIE/iree-amd-aie/Transforms/AMDAIEFunctionOutlining.cpp Outdated Show resolved Hide resolved
compiler/plugins/target/AMD-AIE/iree-amd-aie/Transforms/AMDAIEInsertLoopsForVectorization.cpp Show resolved Hide resolved
@erwei-xilinx
Copy link
Contributor

Nice progress! This PR needs a bit more polish. Also please talk with @erwei-xilinx and see if he would help to make AIR tests work with the changes.

Thanks for the heads up, @yzhang93! WiP on AIR support: #857.

@Abhishek-Varma Abhishek-Varma mentioned this pull request Oct 29, 2024
Merged
@Abhishek-Varma
Copy link
Contributor Author

Nice progress! This PR needs a bit more polish. Also please talk with @erwei-xilinx and see if he would help to make AIR tests work with the changes.

Thanks for the heads up, @yzhang93! WiP on AIR support: #857.

Thank you so much @erwei-xilinx for the fix! ❤️

I took the changes from Xilinx/mlir-air#752 (created by Erwei) and applied to local mlir-air submodule.

I confirm all the tests I commented in this PR works with Erwei's fix.

@Abhishek-Varma Abhishek-Varma changed the base branch from main to avarma_create_func_outline November 4, 2024 14:07
@Abhishek-Varma Abhishek-Varma force-pushed the avarma_create_func_outline branch from 1158090 to d0c0c34 Compare November 4, 2024 14:08
@Abhishek-Varma
Copy link
Contributor Author

Hi @jtuyls @yzhang93 @newling - could you please re-review this PR now that #862 is close to being merged ?

@Abhishek-Varma Abhishek-Varma requested a review from jtuyls November 5, 2024 09:36
Abhishek-Varma added a commit that referenced this pull request Nov 5, 2024
@Abhishek-Varma
[LinalgFunctionOutlining] Create a pass to outline linalg compute ops (…
fded307 
…#862)

This PR creates a standalone `iree-amdaie-linalg-function-outlining`
pass which will be used to later enable the [larger Matmul + Truncf
e2e](#856).

Signed-off-by: Abhishek Varma <abhvarma@amd.com>
Base automatically changed from avarma_create_func_outline to main November 5, 2024 11:44
jtuyls
jtuyls approved these changes Nov 5, 2024
View reviewed changes
Copy link
Collaborator

@jtuyls jtuyls left a comment

Choose a reason for hiding this comment

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

LGTM

@Abhishek-Varma
[Insert-Loops-For-Vec] Update insert-loops-for-vectorization pass
e809fc0 
-- This commit makes the following updates to insert-loops-for-vectorization
   pass.
-- It makes it to work on bufferized inputs.
-- It also involves update pertaining to collapsing unit dimensions of
   a candidate generic op.
-- Also involves coalescing of the loops generated for tiles.
-- This is the first logically grouped PR needed to make Matmul + Truncf work
   for larger shape, and also unblock other outstanding/dependent PR like
   #846

Signed-off-by: Abhishek Varma <abhvarma@amd.com>
@Abhishek-Varma
[WIP] Enable vectorization after bufferization (insert-cores)
b471c84
@Abhishek-Varma
Fix conv pipeline failure + temporarily remove insert_loops test
42a2460
@Abhishek-Varma
Reenable insert-loops test
d1765e4
@Abhishek-Varma
Fix more AIR e2e
36f329d
@Abhishek-Varma
Add lit test for subview->vector.transfer_read/transfer_write
1402a8d
@Abhishek-Varma
Add e2e test for larger Martmul + truncf
d14ecfa
@Abhishek-Varma
Fix canonicalization for trivial access read/write subview->vec
a93fd8f
@Abhishek-Varma
Fix e2e test
fd53e41
@Abhishek-Varma
Nit change
8a72f65
@Abhishek-Varma
Review comments for Canonicalization pattern
099e4a1
@Abhishek-Varma
Review comments InsertLoops
a68a7de
@Abhishek-Varma
Add e2e lit test of Matmul + Truncf
8b4ce90
@Abhishek-Varma
Nit change
37c1a3f
@Abhishek-Varma
Add linalg-function-outlining pass
18545ea
@Abhishek-Varma
Review comment
38d2f48
@newling
Copy link
Contributor

newling commented Nov 5, 2024

@Abhishek-Varma PR looks good other than my small remaining comments. My main concern is that vectorization is being disabled for convolution, hopefully that's not necessary after rebasing?

@Abhishek-Varma
Copy link
Contributor Author

@Abhishek-Varma PR looks good other than my small remaining comments. My main concern is that vectorization is being disabled for convolution, hopefully that's not necessary after rebasing?

Hi @newling - yes, this was an err because of rebasing - I've removed the disabling of vectorization for conv now.

@Abhishek-Varma Abhishek-Varma merged commit 12f0502 into main Nov 5, 2024
5 checks passed
@Abhishek-Varma Abhishek-Varma deleted the avarma_func_outline_pr branch November 5, 2024 20:05
@jtuyls jtuyls mentioned this pull request Nov 7, 2024
Open
This was referenced Nov 11, 2024
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jtuyls jtuyls jtuyls approved these changes

@yzhang93 yzhang93 yzhang93 approved these changes

@newling newling newling approved these changes

@makslevental makslevental Awaiting requested review from makslevental makslevental is a code owner

@MaheshRavishankar MaheshRavishankar Awaiting requested review from MaheshRavishankar MaheshRavishankar is a code owner

@nirvedhmeshram nirvedhmeshram Awaiting requested review from nirvedhmeshram nirvedhmeshram is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@Abhishek-Varma @erwei-xilinx @newling @yzhang93 @jtuyls