KULeuven-MICAS / snax_cluster

A heterogeneous accelerator-centric compute cluster
Apache License 2.0
10 stars 9 forks source link

add gemmx prameter support #327

Closed xiaoling-yi closed 1 month ago

xiaoling-yi commented 1 month ago

In this PR, we add the parameter support for gemmx. More specifically, we:

xiaoling-yi commented 1 month ago

Would it be more useful though that there is a default setting for the GeMM config?

I don't know. Maybe it's better to ensure that people explicitly set the PE size? The streamer parameters should be matched with the PE array size

rgantonio commented 1 month ago

I don't know. Maybe it's better to ensure that people explicitly set the PE size? The streamer parameters should be matched with the PE array size

I'm not too picky so okay haha. Maybe we can add somekind of documentation on our documentation page for this 😄

xiaoling-yi commented 1 month ago

I don't know. Maybe it's better to ensure that people explicitly set the PE size? The streamer parameters should be matched with the PE array size

I'm not too picky so okay haha. Maybe we can add somekind of documentation on our documentation page for this 😄

We can!

xiaoling-yi commented 1 month ago

For this specific PR though:

Just so you know, the transpose extension only works for 512 data width for readers.

Note: the pipelined SIMD only works for meshRow * meshCol = 64 cases so if you set pe array size that doesn't have 64 output, please take care to set with_pipeline = false.

Can you please please please verify this somewhere in software instead of just this comment

I verified locally. Later relevant documentation will be added to push a warning on this.

Who sets TRANSPOSE_EXTENSION_ENABLE in the software makefile? Is there an automatic way this is set?

The streamer header file gen will set it if a transpose extension is instantiated.