dialects: (riscv) Add HasInsntrait and implement trait for some operations

[AntonLydike]

In order to make progress on #2468 I factored out the insn representation. This will hopefully also make it usable for the RISC-V backend efforts.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 89.79%. Comparing base (9d9bbdd) to head (d73b3c8).
Report is 16 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2784      +/-   ##
==========================================
- Coverage   89.80%   89.79%   -0.01%     
==========================================
  Files         381      388       +7     
  Lines       48226    48548     +322     
  Branches     7377     7445      +68     
==========================================
+ Hits        43308    43595     +287     
- Misses       3765     3785      +20     
- Partials     1153     1168      +15     

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

[jorendumoulin]

If these functions are still implemented as risc-v ops, does that not undo the progress of pr #2784 ? As these ops operate on riscv-register operands in comparison to just i32 or index variables, we will have to do an arith-to-riscv pass anyway or insert a lot of UnrealizedConversionCastOps instead. Why not implement them as operations in the snrt dialect (where the operands are not risc-v registers) where this trait could be used?

[AntonLydike]

If these functions are still implemented as risc-v ops, does that not undo the progress of pr #2784 ?

I think you linked to the wrong PR here.

As these ops operate on riscv-register operands in comparison to just i32 or index variables, we will have to do an arith-to-riscv pass anyway or insert a lot of UnrealizedConversionCastOps instead.

I don't know if I can quite follow. The riscv backend of @superlopuh is already able to convert arith to riscv dialect, but for the snax work we want to go through the llvm dialect, to which arith can also already be lowered.

Why not implement them as operations in the snrt dialect (where the operands are not risc-v registers) where this trait could be used?

Because snrt dialect ops map to multiple individual instructions, making it impractical to carry this information imo.

With this plan we gain both:

• A more flexible conversion pattern that converts riscv-to-llvm
• The ability to convert riscv operations to .insns further down the line in the riscv backend, so that that project can move away from a custom riscv toolchain as well.

[superlopuh]

What would be the best way to test this?

[jorendumoulin]

I have tested this in verilator simulations, and once this is in we can convert the tests in ci to use this system over at snax-mlir.
Without simulations, I'm not sure.

[superlopuh]

Here a pytest test that checks that these operations have the exact string you would expect would be good, just to make sure that something in the formatting infrastructure doesn't break, for example

[jorendumoulin]

Okay, here again I wast just a bit worried about the unrealized conversian cast resolution

[AntonLydike]

Pyright complains that: Method "get_insn" cannot be called because it is abstract and unimplemented (reportGeneralTypeIssues)

But get_insn is an abstract method on HasInsnRepresentation, which itself is ABC, so I can never get it "not implemented" as I will always get a subclass of this trait (that by definition has to implement the method). How the heck am I supposed to fix this? @superlopuh @math-fehr

[superlopuh]

I think it's because the get_insn type parameter is invariant, meaning it says that it returns only instances of this type exactly, not subclasses. I think we want the covariant version instead.

[superlopuh]

Turns out I was wrong, it was indeed a Pyright bug