transformations: (convert-riscv-to-llvm)

[jorendumoulin]

This PR implements a conversion of riscv dialect to llvm using the inline assembly op.
This is useful such that any generated RISC-V code can also be used with an LLVM target like we use in snax-mlir.
Specifically, this will be used to reuse the lowering of snrt to riscv, without having to duplicate the entire snrt-to-riscv transform.

@AntonLydike

[jorendumoulin]

Some further reflection on this approach:

It is functional, but the amount of unrealized conversion casts gets a bit ridiculous (see KULeuven-MICAS/snax-mlir#116). Also, the emission of riscv arithmetic in llvm inline assembly instead of regular llvm ir is a bit silly (not in the example).

I think the following approach would make more sense:

Create ops for the custom snitch operations (such as dmdst, dmsrc, dmstrd, ...) in the snrt dialect as snrt primitives, which still operate on basic mlir types (i32, i64) as opposed to riscv registers.

refactor the convert-snrt-to-riscv pass to first lower to the snrt primitives, and only later convert them to actual riscv operations. This makes the alternative path to llvm much easier and as such we can also avoid the unrealized conversion casts entirely.

The lowerings would look like this:

              snrt                                                   
              │                                                      
              │ lower-snrt                                           
              │                                                      
              │                                                      
              ▼                                                      
              arith + snrt primitives                                
                              │                                      
              │               │                                      
              │               │                                      
   ┌──────────┘               └───────┐                              
   │  convert-snrt-to-riscv           │  convert-snrt-to-llvm        
   │  convert-arith-to-riscv          │  convert-arith-to-llvm (MLIR)
   ▼                                  ▼                              
riscv                          llvm + llvm inline asm                

I can work on this, but I would like to get your opinion on this first @AntonLydike @JosseVanDelm .

[JosseVanDelm]

@jorendumoulin I agree with your suggestion.
Implementing the regular arithmetic directly as riscv operations feels like premature lowering, I think we have support for those arith to riscv lowerings in xdsl anyway.

CC @cappadokes

[cappadokes]

Flattering your tag as may be @JosseVanDelm, I find Joren's mentioning of Anton much more meaningful. I don't have the knowledge to constructively comment on this discussion--but thanks again for considering me.

[AntonLydike]

Oh no, we hit the magic interactive tests again :(

[AntonLydike]

@jorendumoulin I did a pass to handle named registers in puts differently. We now just print the register name in the assembly instead of going through the SSA value.

Additionally, with #2797 we can now CSE delete the remaining reg_register ops left over by this pass.

I'll fix the problem with ops that return a riscv.reg<zero> register tomorrow, and that should hopefully make this PR complete.

[superlopuh]

What's the latest on this?

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 90.19%. Comparing base (c5a1c4e) to head (eb4b558).
Report is 6 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2468      +/-   ##
==========================================
+ Coverage   90.18%   90.19%   +0.01%     
==========================================
  Files         456      458       +2     
  Lines       57567    57705     +138     
  Branches     5541     5560      +19     
==========================================
+ Hits        51917    52049     +132     
- Misses       4191     4194       +3     
- Partials     1459     1462       +3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[superlopuh]

No longer [WIP]?

[AntonLydike]

Yes, this is now ready for your all eyes

[jorendumoulin]

Indeed, no longer WIP

Thank you, this looks good to me!

[superlopuh]

@jorendumoulin, I think if you could update this and get it in, it would be fabulous, I'm sure we'll find more use-cases for this functionality

[jorendumoulin]

I rebased and resolved all outstanding comments, I just don't really get what is meant with this final comment:

won't this break when returning 0?

[superlopuh]

Why not index type?

[jorendumoulin]

llvm dialect doesn't like index type

[superlopuh]

!!

[superlopuh]

I rebased and resolved all outstanding comments, I just don't really get what is meant with this final comment:

won't this break when returning 0?

I think it may have been outdated, before the get registers for zero... I'm not 100% sure, I can no longer spot an issue.

[jorendumoulin]

Thanks! Apologies for the extreme delay on this! I hope I'll soon find some time to convert our snitch runtime stuff to use this flow :)

[superlopuh]

The important thing is that we got there in the end, thank you very much for picking it back up!