Need help on interchangeLoops

MLIR
1

Hello, I don’t quite understand how mlir::interchangeLoops works. The input loop is:

affine.for %arg0 = 0 to 100 { // outer loop
affine.for %arg1 = 0 to 300 { // middle loop
affine.for %arg2 = 0 to 200 { // inner loop

Then, I perform the interchange using:

SmallVector<unsigned,3> index_map ;
SmallVector<AffineForOp,3> loop_nest ;
index_map.push_back(2) ;
index_map.push_back(1) ;
index_map.push_back(0) ;
loop_nest.push_back(outer_loop) ;
loop_nest.push_back(middle_loop) ;
loop_nest.push_back(inner_loop) ;
mlir::interchangeLoops(llvm::makeArrayRef(loop_nest),llvm::makeArrayRef(index_map)) ;

In the end I get the loop nest:

affine.for %arg0 = 0 to 300 {
  affine.for %arg1 = 0 to 200 {
    affine.for %arg2 = 0 to 100 {

The permutation is quite different from what I expected, which is:

affine.for %arg0 = 0 to 200 {  // original index 2 goes to 0
  affine.for %arg1 = 0 to 300 {// original index 1 goes to 1
    affine.for %arg2 = 0 to 100 {// original index 0 goes to 2

Can you explain me why it’s not what I expected?

2

I hit the same issue a few weeks ago. I don’t remember the details but I think loop interchange expects the loops to be provided in inner-to-outer order. Otherwise, it won’t do it correctly. You can try:

loop_nest.push_back(inner_loop) ;
loop_nest.push_back(middle_loop) ;
loop_nest.push_back(outer_loop) ;

Probably Andy or @bondhugula can correct me if I’m wrong.

3

This clearly looks like a bug and missing / inaccurate documentation as well. Thanks for reporting. mlir::interchangeLoops is also missing assertions on the nesting check as well as on the size of loops. It also does not document what it returns. Could you please file a bug?

4

On a side note, you don’t need to use makeArrayRef; SmallVector will implicitly convert. Also, SmallVector<unsigned, 3> indexMap = {0, 1, 2}; will work.

5

I noted that. I just thought at some point that it’s the lack of conversion that may pose a problem…

6

Btw, if you use {0, 1, 2}, then the expected output is no interchange at all. Note that 0 corresponds to outermost, and 2 to innermost.

7

No, I used {2,1,0} (corrected it now in the post, the rest is correct). I was just tired yesterday night when I posted.

The bug is in file LoopUtils.cpp in function

mlir::interchangeLoops(ArrayRef<AffineForOp>, loops,ArrayRef<unsigned> loopPermMap)

For my permutation, the code will only sink by 2 for i==0, which gives exactly the result provided by the code, but not the good permutation.

Formally, the code must decompose the permutation into rotations (sinkLoop), but the code is incorrect because it does not update the loopPermMap while rotations are performed, hence the bug.

I don’t know how to submit a bug. Until now there were the “issues”, but they are now gone…

8

Hello. The order is correct. Using the order you propose gives an error (exactly because you have to follow an order, something which I did not find in the documentation).

9

Bugs are now to be reported at https://bugs.llvm.org (select MLIR as the ‘product’).

1 Like
10

It will wait a bit, as automatic registration is disabled.

11

D77004 fixes this bug.

12

Ok, thanks, I’ll wait until it’s included in the main repo. Normally, how much time does it take?

13

As soon as someone reviews and approves it, I can commit it. It shows up in the main repo instantaneously. You are free to review it as well!