dialects: Implement MemoryEffect on RISCV op, just reuse main CSE. #2685
Conversation
PapyChacal
added
dialects
| "test.op"(%d8, %e8) : (!riscv.reg<>, !riscv.reg<>) -> () | ||
| } | ||
|
|
||
| %f8 = riscv.li 8 : () -> !riscv.reg<> | ||
|
|
||
| "test.op"(%a8, %b8, %c8, %a7, %a7, %b7, %f8) : (!riscv.reg<>, !riscv.reg<>, !riscv.reg<>, !riscv.reg<>, !riscv.reg<>, !riscv.reg<>, !riscv.reg<>) -> () | ||
|
|
There was a problem hiding this comment.
Added two test.ops to use value and prevent CSE to rightfully CSE away the whole module
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2685 +/- ##
==========================================
- Coverage 89.79% 89.77% -0.02%
==========================================
Files 384 385 +1
Lines 48442 48490 +48
Branches 7429 7436 +7
==========================================
+ Hits 43497 43534 +37
- Misses 3781 3786 +5
- Partials 1164 1170 +6 ☔ View full report in Codecov by Sentry. |
Left as an exercise for the reader I guess; I see this is enough to do what's tested, but I don't know every corner of this dialect stack. Feel free to add the trait to any missing operation you see!
What do you mean by that? If you mean modelling read/write effects in more detail, yes, this is left as further work. I can help with if it would be useful somewhere specifically? |
|
I would say that this is good to merge as-is, in an incomplete state, we can fill in the rest of the ops with the effect later. |
Alright, will update and merge soonish 🙂 |
Co-authored-by: Sasha Lopoukhine <superlopuh@gmail.com>
| @@ -75,7 +80,6 @@ riscv_scf.for %13 : !riscv.reg = %11 to %8 step %12 { | |||
| // CHECK-NEXT: %{{.*}} = riscv_scf.for %{{.*}} : !riscv.reg = %{{.*}} to %{{.*}} step %{{.*}} iter_args(%{{.*}} = %{{.*}}) -> (!riscv.freg) { | |||
| // CHECK-NEXT: %{{.*}} = riscv_snitch.read from %{{.*}} : !riscv.freg | |||
| // CHECK-NEXT: %{{.*}} = riscv_snitch.read from %{{.*}} : !riscv.freg | |||
| // CHECK-NEXT: %{{.*}} = riscv.fmul.d %{{.*}}, %{{.*}} : (!riscv.freg, !riscv.freg) -> !riscv.freg | |||
We called and this was about a misread of the test-case: An unused operation was correctly erased, the example simply intended to have it used instead. |
Remove the
riscv-csepass, instead implement AllocatedMemoryEffect on some RISCV ops and let generic CSE do the job.This makes CSE more powerful, with dead-code elimination and common subexpression elimination across regions.