`scf.while` -> `scf.for` uplifting as canonicalization

MLIR
1

I’ve recently added transformation and pass to uplift scf.while to scf.for ([mlir][scf] Uplift `scf.while` to `scf.for` by Hardcode84 · Pull Request #76108 · llvm/llvm-project · GitHub) and there was some discussion about making it part of scf.while canonicalization instead of standalone transformation (see comments in PR).

I want to collect peoples opinions on this topic. Personally, I’m fine to have it standalone and there are some potential issues with having it as canonicalization:

Also, it will need one more pattern which probably doesn't qualify as canonicalization - duplicate and move ops from before block - to loop body and after, e.g:

scf.while(...) {
before:
  ...
  some_op()
  scf.condition ..
after:
  ...
}

to

scf.while(...) {
before:
  ...
  scf.condition ..
after:
  some_op()
  ...
}
some_op()

CC @mehdi_amini @stellaraccident

1 Like
2

+1, this does not seem like a general canonicalization and won’t save us any bitflips AFAICT. Also it’s not unreasonable to have lowering that goes the opposite way.

If we zoom out a bit, it might be useful to have a meta-discussion on what should we consider as canonicalizations. I wrote something on this a few months ago, but it’s more of a brain dump and not fully fleshed out: How we canonicalize - Google Docs

3

This isn’t unusual to me that canonicalization and lowering don’t agree with each other: this happens “by design”: in general you need to move out of the “canonical” form while lowering (LLVM has the “codegen prepare” phase for this for example, where you can’t run arbitrary passes before getting into instruction selection).

These don’t look like canonicalization and as such shouldn’t be added there, but that seems a bit orthogonal to the question of forming a for-loop when possible though?

There is also the cost of matching, I’m not sure how costly your logic is right now.

4

Yes, but it means we will need a dedicated UpliftPrepare pass before and we can just make uplifting part of such pass instead. Also, if it’s not a canonical form, someone may add a canonicalization which undo such preparation and it will break us as we do uplift as part of canonicalization. (side note: it also needs LICM as preparation)

I don’t have any measurements, but It does some dominance checks and also it iterates over uses in one place for which it may be possible to construct pathological case.

5

Sure, but that is still orthogonal to me: the fact is that the form that can be uplifted can appear during canonicalization (because of other foldings for example), and the pattern can still apply then! This isn’t meant to replace your uplifting pass of course.

Right, I’d be more concerned about this: canonicalization is a process which can repeat the attempt to apply the patterns over and over, so a “failing match” should be as cheap as possible.

6

Do you mean adding same uplifting pattern both to canonicalization and to the dedicated uplifting pass? Yes, this will work

It iterates over uses during match phase, so it can do it repeatedly in pathological case, but after you mentioned it, I think it may be possible to get rid of uses iteration.

7
8

Preparation pattern