transformations: convert type_offsets in ptr to arith.constant
#3394
Conversation
| @@ -0,0 +1,90 @@ | |||
| // RUN: xdsl-opt -p convert-memref-to-ptr,convert-ptr-type-offsets --print-op-generic --split-input-file %s | mlir-opt --allow-unregistered-dialect --mlir-print-op-generic -pass-pipeline 'builtin.module(func.func(scf-for-loop-canonicalization,scf-for-loop-range-folding,scf-for-loop-canonicalization))' | xdsl-opt -p scf-for-loop-flatten --print-op-generic | mlir-opt --allow-unregistered-dialect --mlir-print-op-generic -pass-pipeline 'builtin.module(func.func(scf-for-loop-canonicalization,scf-for-loop-range-folding,scf-for-loop-canonicalization))' | |||
There was a problem hiding this comment.
could you please try using the mlir opt pass in xDSL? should simplify the pipeline a little bit
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #3394 +/- ##
==========================================
+ Coverage 90.13% 90.18% +0.04%
==========================================
Files 452 455 +3
Lines 57157 57282 +125
Branches 5498 5504 +6
==========================================
+ Hits 51519 51657 +138
+ Misses 4180 4173 -7
+ Partials 1458 1452 -6 ☔ View full report in Codecov by Sentry. |
|
It would be great to add an individual filecheck test for the pass also |
| @@ -0,0 +1,79 @@ | |||
| // RUN: xdsl-opt -p convert-memref-to-ptr,convert-ptr-type-offsets,mlir-opt[scf-for-loop-canonicalization,scf-for-loop-range-folding,scf-for-loop-canonicalization],scf-for-loop-flatten,mlir-opt[scf-for-loop-canonicalization,scf-for-loop-range-folding,scf-for-loop-canonicalization] --split-input-file %s | filecheck %s | |||
There was a problem hiding this comment.
How much are we missing in xDSL from the mlir-opt pipeline here? If it's not a lot, I'd much rather have this logic in xDSL.
There was a problem hiding this comment.
I believe both scf-for-loop-canonicalization and scf-for-loop-range-folding are missing. Do you think it's worth porting them?
There was a problem hiding this comment.
Yep definitely, I'd love to keep the Snitch compilation flow entirely working without the need of MLIR, for environments like WASM, and for when we start the work of schedule exploration, and whatever hackery we need to do to make it fast, it's much easier to play with all this in one environment.
There was a problem hiding this comment.
scf-for-loop-range-folding is already in, and was less buggy than the MLIR one until recently, where I upstreamed a bug fix to MLIR after noticing the difference with xDSL :)
There was a problem hiding this comment.
How about we bench it first? I'm just curious to see if it makes any improvements. Plus, if there is no speed increase, maybe the port won't be worth it.
There was a problem hiding this comment.
Oh, right, there is. But I think it's only for the riscv loops. Do we want a general scf version?
There was a problem hiding this comment.
I can guarantee speed improvements, and register pressure improvements in the final assembly, if that's what you mean
There was a problem hiding this comment.
yep, I think it would be worth having an scf version for this
There was a problem hiding this comment.
ok, I'll port them to xdsl
There was a problem hiding this comment.
Do we now have everything we need?
It seems like having a separate test for this is a little wasteful, so I decided to test it in loop folding.