dialects: (riscv) initial arith lowering

[adutilleul]

• Lowering scalar types (i.e: not Vector)
• Lowering for ALU-like ops
• Lowering for FPU-like ops
• Tests

[codecov]

Codecov Report

Patch coverage: 98.30% and project coverage change: +0.11 🎉

Comparison is base (0b866aa) 88.66% compared to head (7d29395) 88.77%.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1212      +/-   ##
==========================================
+ Coverage   88.66%   88.77%   +0.11%     
==========================================
  Files         167      168       +1     
  Lines       22874    23110     +236     
  Branches     3480     3506      +26     
==========================================
+ Hits        20281    20517     +236     
+ Misses       2038     2032       -6     
- Partials      555      561       +6     

Impacted Files	Coverage Δ
...dsl/backend/riscv/lowering/riscv_arith_lowering.py	98.29% <98.29%> (ø)
xdsl/xdsl_opt_main.py	96.65% <100.00%> (+0.01%)	⬆️

... and 1 file with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

[superlopuh]

Should probably use the GreedyRewritePatternApplier here

[superlopuh]

I think this is ready to merge modulo greedy pattern applier

[JosseVanDelm]

Hi @adutilleul,

Thanks for your contribution! This is an important step forward, but I do have some questions

I missed the part where we introduced signed integers to the Riscv dialect (or I forgot that I did that? Not sure anymore).
After learning about signless types it seems a bit weird to me that we would be introducing them after arith, since riscv assembly is pretty much signless all the way?

I also don't like the naming of the current pass.

Also, this PR introduces a lot of exceptions that don't seem to be tested.
Can you add some negative tests to increase coverage please?

Thanks!

[JosseVanDelm]

Forgot to press "request changes" on my last review!

[superlopuh]

I don't think we need the tests for coverage, seems like an inversion of the purpose of coverage. It seems that the implemented lowerings are tested in the filecheck, so I think we're good. About the name, I don't think we're that careful about non platform-specific lowerings elsewhere, I don't think it's worth worrying about that too much for the time being. At some point we'll come up with actual infrastructure for targets and stick to it, but I'd rather just assume that riscemu is the only riscv that exists until that becomes a problem.

[adutilleul]

I've changed the naming @JosseVanDelm, so I think we're good for merging ?

[JosseVanDelm]

@adutilleul sure! go ahead! sorry for the slow response!