One column Matrix Multiplication for Ryzen AI #859

jgmelber · 2023-12-12T17:41:34Z

No description provided.

jgmelber · 2023-12-12T17:42:21Z

@andrej

github-actions

⚠️ `Clang-Tidy` found issue(s) with the introduced code (1/8)

reference_designs/ipu-xrt/matrix_multiplication_column/test.cpp

github-actions

⚠️ `Clang-Tidy` found issue(s) with the introduced code (2/8)

reference_designs/ipu-xrt/matrix_multiplication_column/test.cpp

github-actions

⚠️ `Clang-Tidy` found issue(s) with the introduced code (3/8)

reference_designs/ipu-xrt/matrix_multiplication_column/test.cpp

github-actions

⚠️ `Clang-Tidy` found issue(s) with the introduced code (4/8)

reference_designs/ipu-xrt/matrix_multiplication_column/test.cpp

github-actions

⚠️ `Clang-Tidy` found issue(s) with the introduced code (5/8)

reference_designs/ipu-xrt/matrix_multiplication_column/test.cpp

reference_designs/ipu-xrt/matrix_multiplication_column/zero.cc

github-actions

⚠️ `Clang-Tidy` found issue(s) with the introduced code (6/8)

reference_designs/ipu-xrt/matrix_multiplication_column/zero.cc

reference_designs/ipu-xrt/matrix_multiplication_column/mm.cc

github-actions

⚠️ `Clang-Tidy` found issue(s) with the introduced code (7/8)

reference_designs/ipu-xrt/matrix_multiplication_column/mm.cc

github-actions

⚠️ `Clang-Tidy` found issue(s) with the introduced code (8/8)

reference_designs/ipu-xrt/matrix_multiplication_column/mm.cc

github-actions · 2023-12-12T17:53:50Z

Coverage Report

Created: 2023-12-20 08:31

Click here for information about interpreting this report.

Filename	Function Coverage	Line Coverage	Region Coverage	Branch Coverage
Totals	-	-	-	-

Generated by llvm-cov -- llvm version 14.0.0

andrej

It's really cool to see this in Python, Joe. Much cleaner than with the templates. I know you didn't request a review from me but I hope you don't mind that I left some comments. Feel free to ignore them. The comment I added to the C kernel file is the only one I feel strongly about.

reference_designs/ipu-xrt/matrix_multiplication_column/Makefile

reference_designs/ipu-xrt/matrix_multiplication_column/README.md

reference_designs/ipu-xrt/matrix_multiplication_column/aie2.py

reference_designs/ipu-xrt/matrix_multiplication_column/mm.cc

reference_designs/ipu-xrt/matrix_multiplication_column/aie2.py

makslevental · 2023-12-13T05:22:10Z

@jgmelber n00b question: is it not possible to write the kernel code (stuff in mm.cc) in python as well? E.g, doesn't aievec exist and have a lowering to cpp that can emit roughly the same kind of stuff? Or are there lots of things missing from the aievec dialect that you therefore (today) have to write by hand (in cpp).

jgmelber · 2023-12-13T17:12:33Z

@jgmelber n00b question: is it not possible to write the kernel code (stuff in mm.cc) in python as well? E.g, doesn't aievec exist and have a lowering to cpp that can emit roughly the same kind of stuff? Or are there lots of things missing from the aievec dialect that you therefore (today) have to write by hand (in cpp).

As far as I understand there are still some missing pieces. But verifying the performance of aievec is something that needs to be done

reference_designs/ipu-xrt/matrix_multiplication_column/mm.cc

github-actions · 2023-12-19T11:17:16Z

reference_designs/ipu-xrt/matrix_multiplication_column/mm.cc

+
+#define zero_scalar_c_func(ctype_in, mlir_type_in, ctype_out, mlir_type_out,   \
+                           r, s, t)                                            \
+  void zero_scalar_##mlir_type_out((ctype_out) *c_out) {                         \


[clang-format] _{reported by reviewdog 🐶}

Suggested change

void zero_scalar_##mlir_type_out((ctype_out) *c_out) { \

void zero_scalar_##mlir_type_out((ctype_out) * c_out) { \

reference_designs/ipu-xrt/matrix_multiplication_column/test.cpp

reference_designs/ipu-xrt/matrix_multiplication_column/aie2.py

AndraBisca · 2023-12-19T11:51:35Z

Thank you for all of your comments @andrej, they are really helpful!
While I didn't resolve all of them, I think it might be best to merge this PR for now to avoid accumulating conflicts with main. @jgmelber can make any subsequent changes in another update-PR.

github-actions

⚠️ `Clang-Tidy` found issue(s) with the introduced code (1/1)

reference_designs/MM_2x2/objectFifo_circuit_switched_version/test.cpp

reference_designs/MM_2x2/packet_switched_version/test.cpp

reference_designs/MM_2x2/circuit_switched_version/test.cpp

github-actions

⚠️ `Clang-Tidy` found issue(s) with the introduced code (1/3)

reference_designs/MM_2x2/packet_switched_version/test.cpp

reference_designs/MM_2x2/objectFifo_circuit_switched_version/test.cpp

github-actions

⚠️ `Clang-Tidy` found issue(s) with the introduced code (2/3)

reference_designs/MM_2x2/objectFifo_circuit_switched_version/test.cpp

reference_designs/MM_2x2/circuit_switched_version/test.cpp

github-actions

⚠️ `Clang-Tidy` found issue(s) with the introduced code (3/3)

reference_designs/MM_2x2/circuit_switched_version/test.cpp

makslevental · 2023-12-20T07:30:39Z

Heads up @AndraBisca I marked reference_designs/autocorrelation/objectFifo_version/aie.mlir as XFAIL because it uses old objectfifo syntax that I can't quite patch up.

AndraBisca · 2023-12-20T13:07:31Z

Heads up @AndraBisca I marked reference_designs/autocorrelation/objectFifo_version/aie.mlir as XFAIL because it uses old objectfifo syntax that I can't quite patch up.

Thank you for the heads-up @makslevental. I'll take a look at it very soon, along with all the other reference designs and clean them up.

jgmelber requested review from jamesroxbypb and denolf December 12, 2023 17:42

github-actions bot reviewed Dec 12, 2023

View reviewed changes

andrej reviewed Dec 12, 2023

View reviewed changes

makslevental force-pushed the matmul branch 2 times, most recently from 5bc8d53 to 5b5f441 Compare December 13, 2023 05:00

Xilinx deleted a comment from github-actions bot Dec 13, 2023

github-actions bot reviewed Dec 13, 2023

View reviewed changes

github-actions bot reviewed Dec 19, 2023

View reviewed changes

github-actions bot reviewed Dec 20, 2023

View reviewed changes

reference_designs/MM_2x2/objectFifo_circuit_switched_version/test.cpp Outdated Show resolved Hide resolved

reference_designs/MM_2x2/packet_switched_version/test.cpp Outdated Show resolved Hide resolved

reference_designs/MM_2x2/circuit_switched_version/test.cpp Outdated Show resolved Hide resolved

makslevental force-pushed the matmul branch 2 times, most recently from 95ba3ab to 4f19d28 Compare December 20, 2023 06:37

github-actions bot reviewed Dec 20, 2023

View reviewed changes

makslevental force-pushed the matmul branch 11 times, most recently from 8f67aef to a48f16e Compare December 20, 2023 07:16

makslevental force-pushed the matmul branch 7 times, most recently from f73ce9e to a84809d Compare December 20, 2023 08:23

fix cpp

8c83263

makslevental force-pushed the matmul branch from a84809d to 8c83263 Compare December 20, 2023 08:27

makslevental merged commit e6831a1 into main Dec 20, 2023
32 checks passed

makslevental deleted the matmul branch December 20, 2023 08:46

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

One column Matrix Multiplication for Ryzen AI #859

One column Matrix Multiplication for Ryzen AI #859

jgmelber commented Dec 12, 2023

jgmelber commented Dec 12, 2023

github-actions bot left a comment

github-actions bot left a comment

github-actions bot left a comment

github-actions bot left a comment

github-actions bot left a comment

github-actions bot left a comment

github-actions bot left a comment

github-actions bot left a comment

github-actions bot commented Dec 12, 2023 •

edited

Loading

andrej left a comment

makslevental commented Dec 13, 2023

jgmelber commented Dec 13, 2023

github-actions bot Dec 19, 2023

AndraBisca commented Dec 19, 2023

github-actions bot left a comment

github-actions bot left a comment

github-actions bot left a comment

github-actions bot left a comment

makslevental commented Dec 20, 2023

AndraBisca commented Dec 20, 2023

	void zero_scalar_##mlir_type_out((ctype_out) *c_out) { \
	void zero_scalar_##mlir_type_out((ctype_out) * c_out) { \

One column Matrix Multiplication for Ryzen AI #859

One column Matrix Multiplication for Ryzen AI #859

Conversation

jgmelber commented Dec 12, 2023

jgmelber commented Dec 12, 2023

github-actions bot left a comment

Choose a reason for hiding this comment

⚠️ Clang-Tidy found issue(s) with the introduced code (1/8)

github-actions bot left a comment

Choose a reason for hiding this comment

⚠️ Clang-Tidy found issue(s) with the introduced code (2/8)

github-actions bot left a comment

Choose a reason for hiding this comment

⚠️ Clang-Tidy found issue(s) with the introduced code (3/8)

github-actions bot left a comment

Choose a reason for hiding this comment

⚠️ Clang-Tidy found issue(s) with the introduced code (4/8)

github-actions bot left a comment

Choose a reason for hiding this comment

⚠️ Clang-Tidy found issue(s) with the introduced code (5/8)

github-actions bot left a comment

Choose a reason for hiding this comment

⚠️ Clang-Tidy found issue(s) with the introduced code (6/8)

github-actions bot left a comment

Choose a reason for hiding this comment

⚠️ Clang-Tidy found issue(s) with the introduced code (7/8)

github-actions bot left a comment

Choose a reason for hiding this comment

⚠️ Clang-Tidy found issue(s) with the introduced code (8/8)

github-actions bot commented Dec 12, 2023 • edited Loading

Coverage Report

Created: 2023-12-20 08:31

Generated by llvm-cov -- llvm version 14.0.0

andrej left a comment

Choose a reason for hiding this comment

makslevental commented Dec 13, 2023

jgmelber commented Dec 13, 2023

github-actions bot Dec 19, 2023

Choose a reason for hiding this comment

AndraBisca commented Dec 19, 2023

github-actions bot left a comment

Choose a reason for hiding this comment

⚠️ Clang-Tidy found issue(s) with the introduced code (1/1)

github-actions bot left a comment

Choose a reason for hiding this comment

⚠️ Clang-Tidy found issue(s) with the introduced code (1/3)

github-actions bot left a comment

Choose a reason for hiding this comment

⚠️ Clang-Tidy found issue(s) with the introduced code (2/3)

github-actions bot left a comment

Choose a reason for hiding this comment

⚠️ Clang-Tidy found issue(s) with the introduced code (3/3)

makslevental commented Dec 20, 2023

AndraBisca commented Dec 20, 2023

⚠️ `Clang-Tidy` found issue(s) with the introduced code (1/8)

⚠️ `Clang-Tidy` found issue(s) with the introduced code (2/8)

⚠️ `Clang-Tidy` found issue(s) with the introduced code (3/8)

⚠️ `Clang-Tidy` found issue(s) with the introduced code (4/8)

⚠️ `Clang-Tidy` found issue(s) with the introduced code (5/8)

⚠️ `Clang-Tidy` found issue(s) with the introduced code (6/8)

⚠️ `Clang-Tidy` found issue(s) with the introduced code (7/8)

⚠️ `Clang-Tidy` found issue(s) with the introduced code (8/8)

github-actions bot commented Dec 12, 2023 •

edited

Loading

⚠️ `Clang-Tidy` found issue(s) with the introduced code (1/1)

⚠️ `Clang-Tidy` found issue(s) with the introduced code (1/3)

⚠️ `Clang-Tidy` found issue(s) with the introduced code (2/3)

⚠️ `Clang-Tidy` found issue(s) with the introduced code (3/3)