search

nod-ai / iree-amd-aie

IREE plugin repository for the AMD AIE accelerator
Apache License 2.0
69 stars 30 forks source link

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

Closed Abhishek-Varma closed 2 weeks ago

Abhishek-Varma commented 4 weeks ago

-- 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

erwei-xilinx commented 3 weeks ago

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: https://github.com/nod-ai/iree-amd-aie/pull/857.

Abhishek-Varma commented 3 weeks ago

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 https://github.com/Xilinx/mlir-air/pull/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 commented 2 weeks ago

Hi @jtuyls @yzhang93 @newling - could you please re-review this PR now that https://github.com/nod-ai/iree-amd-aie/pull/862 is close to being merged ?

newling commented 2 weeks ago

@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 commented 2 weeks ago

@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.