[Mlir-commits] [mlir] [mlir][SCF] Skip dynamic peeling for non-index loops (PR #217909)

[Federico Bruzzone]

FedericoBruzzone wrote:

Thanks for looking into this!

To close the loop on my own question above: it's not fundamentally infeasible, but I don't think it's a drop-in change either. Let's try to think about this:
1. **Bit width**: index is data-layout dependent, so casting a wider (or same-width, if unsigned) integer type down to `index` and back is not always lossless.
2. **Signedness**: `scf.for` supports `scf.for unsigned`, so the choice between `arith.index_cast` and `arith.index_castui` matters.

There's actually already a solution to exactly this sub-problem in-tree for `SCFToAffine`, for both _width/sign guard_:
https://github.com/llvm/llvm-project/blob/b8ba3c2b72cb53268129bbecfeb4ba7ec5b8d831/mlir/lib/Conversion/SCFToAffine/SCFToAffine.cpp#L123-L145
and _sign-aware cast helper_:
https://github.com/llvm/llvm-project/blob/b8ba3c2b72cb53268129bbecfeb4ba7ec5b8d831/mlir/lib/Conversion/SCFToAffine/SCFToAffine.cpp#L310-L315

I prototyped applying the _same guard + cast pattern_ to `peelForLoop` locally, and it seems to work :D
Also, I believe https://github.com/llvm/llvm-project/blob/0d2c1ab5eef5585123e72a3a0243465b396c435b/mlir/lib/Dialect/SCF/Transforms/LoopSpecialization.cpp#L228-L229 also suffers from the same issue, right? :'D

---

That being said, I don't want to block this PR in any way, and thank you again. I think it's a very conservative choice (which isn't necessarily wrong). I think it's best to wait for the opinion of some experts. 
Kind ping: @matthias-springer @banach-space

https://github.com/llvm/llvm-project/pull/217909