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.

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

dialects: merge snitch dialects #2885

Open
superlopuh opened this issue Jul 15, 2024 · 3 comments
Open

dialects: merge snitch dialects #2885

superlopuh opened this issue Jul 15, 2024 · 3 comments
Labels
dialects Changes on the dialects

Comments

@superlopuh
Copy link
Member

superlopuh commented Jul 15, 2024

From experience trying to explain our lowerin in the paper and presentations, it seems that we have too many dialects to explain what's going on. I propose to move the riscv-specific operations to two dialects: snitch and riscv_snitch.

More concretely:

snitch_stream.* -> riscv_snitch
snitch.* -> riscv_snitch

[Edited]

@superlopuh superlopuh added the dialects Changes on the dialects label Jul 15, 2024
@JosseVanDelm
Copy link
Collaborator

I tend to agree with this sentiment, but I'd check in with @jorendumoulin since he was planning to use a similar lowering approach for the snax streamers which is slightly different.
@jorendumoulin what are your thoughts on this?

@jorendumoulin
Copy link
Collaborator

hello yes!
For the first two, it definitely makes sense.
For the third, I am finally doing what I said I would be doing a month a ago and using the memref_stream for SNAX streamers.

It seems our lowering paths are like this:

                    ┌─────────────►  snitch_stream
                    │                             
                    │                             
linalg ────►  memref_stream                       
                                                  
                    │                             
                    │                             
                    └─────────────►  snax_stream  

I think the memref_stream abstraction definitely makes sense and is proving to be very useful for me. If merging the dialects means just moving over the operations, then fine (although memref_stream is quite snitch/risc-v-independent, so I'm not sure if this is logical). If it means actually changing the operations, this would be inconvenient for me.

@superlopuh
Copy link
Member Author

That's interesting and a good point. Thank you very much for commenting on this as this is super useful feedback! In that case I'd probably want to just get rid of the snitch_stream dialect, and roll it into snitch, ending up with memref_stream, snitch, and riscv_snitch, where riscv_snitch has all the operations that can be printed as assembly, and snitch has everything else.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dialects Changes on the dialects
Projects
Status: No status
Development

No branches or pull requests

3 participants