transforms: (convert-snrt-to-riscv) Convert to arith where possible

[AntonLydike]

Replace some arithmetic RISCV operations with arith dialect operations. This is in preperation for another PR.

This shouldn't change the usability of this pass, but makes it much easier to re-use this pass without targeting our own RISC-V backend.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 89.80%. Comparing base (9d9bbdd) to head (ac209a4).
Report is 1 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #2783   +/-   ##
=======================================
  Coverage   89.80%   89.80%           
=======================================
  Files         381      381           
  Lines       48226    48227    +1     
  Branches     7377     7378    +1     
=======================================
+ Hits        43308    43311    +3     
+ Misses       3765     3764    -1     
+ Partials     1153     1152    -1     

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

[superlopuh]

I don't understand, the pass is to riscv, why would it be to arith instead?

[AntonLydike]

I don't understand, the pass is to riscv, why would it be to arith instead?

Some things can only be represented as RISC-V dialect (eg. csr reads/writes), but others are just arithmetic operations. Instead of hand-lowering the arithmetic as well, I think it makes sense to leave that up to the default lowerings that are already in place (also makes CSE and such easier as arith is 100% pure and has more pattern coverage).

Additionally, this allows us to re-use this lowering when going to e.g. llvm dialect, as we only need to convert some select few RISC-V ops to inline assembly, which is the plan in snax mlir.

[superlopuh]

Surely that's a different pass, that converts snrt to arith instead? This doesn't typecheck for me

[jorendumoulin]

I too, am a bit confused. As I very much welcome this change, this pass does not seem to be the correct place for this?
From the snax perspective, we don't want snrt being lowerd directly to risc-v as this dialect is unfortunately not yet usable for us, so a middle step in arith is necessary, but this seems like it mixes the path from snrt -> arith -> llvm and snrt -> riscv in one pass?

What is wrong with original plan from #2468?:

              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                

[AntonLydike]

@jorendumoulin I think this pass fits relatively nicely into the picture of yours, where the snrt primitives are just riscv.csr* and riscv_snrt ops

[AntonLydike]

I have renamed the pass to inline-snrt as it better reflects it's purpose now.

[jorendumoulin]

Allright, however, using the riscv_snrt dialect as primitives still results in some silly i32->riscv-reg->i32 conversions. It does avoid having almost duplicate ops in two different dialects. I'm not sure whats best.

[AntonLydike]

Allright, however, using the riscv_snrt dialect as primitives still results in some silly i32->riscv-reg->i32 conversions. It does avoid having almost duplicate ops in two different dialects. I'm not sure whats best.

Yes, but these will be cleaned up automagically by xdsl when we reconclile casts, this shouldn't be a problem imo