transformations: (scf) loop range folding #3417
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #3417 +/- ##
==========================================
- Coverage 90.16% 90.16% -0.01%
==========================================
Files 455 456 +1
Lines 57467 57553 +86
Branches 5532 5545 +13
==========================================
+ Hits 51815 51890 +75
- Misses 4195 4200 +5
- Partials 1457 1463 +6 ☔ View full report in Codecov by Sentry. |
| new_step.result.name_hint = "new_step" | ||
| op.operands[2] = new_step.result | ||
|
|
||
| new_lb.result.name_hint = "new_lb" | ||
| new_ub.result.name_hint = "new_ub" |
There was a problem hiding this comment.
This is great, but I have a feeling that it might actually make it easier to read if we keep the name hint as the user's name hint, to make it explicit what calculation we're pulling out
| %0 = arith.addi %shift, %i : index | ||
| %1 = arith.muli %0, %mul_shift : index |
There was a problem hiding this comment.
So wrt my comment below, if we give names to these variables, their names will be visible outside of the for loop in the added arithmetic calculations
There was a problem hiding this comment.
Could you please check if that's what you meant?
| new_name_hint = ( | ||
| user.result.name_hint if user.result.name_hint is not None else "new" | ||
| ) |
There was a problem hiding this comment.
This is cute but I worry that it might be a bit confusing if done extensively, like for the new_lb_lb above. I'm now tempted to say that we should not be doing anything here, and let the default name copying done by the rewriter do the work.
There was a problem hiding this comment.
I agree, to be honest. Should I omit name hints here?
There was a problem hiding this comment.
Yep, I would just add the name hints to all variables in the test file, and remove the name hint manipulation in the rewrite
Porting mlir pass of the same name. The logic is almost the same. We need this to keep snitch compilation fully inside xdsl.
Porting mlir pass of the same name. The logic is almost the same. We need this to keep snitch compilation fully inside xdsl.