[NFC][LoopInterchange] Explicitly pass both `InnerLoop` and `OuterLoop` to `processLoop`

[TaWeiTu]

Summary

This is a split patch of D96644.

Explicitly pass both InnerLoop and OuterLoop to function processLoop to remove the need to swap elements in loop list and allow making loop list an ArrayRef.
Also, fix inconsistent spellings of OuterLoopId and Inner Loop Id in debug log.

[fhahn]

The main reason for this refactor is to help passing ArrayRef to function processLoop() (remove the need to swap elements in loop list).

Could you elaborate why this is needed to pass ArrayRef? I think stylistically passing the outer-loop Id explicitly makes the functions a bit clearer and the functions currently are written in a way that does not rely on passing adjacent loops. If possible to keep the parameters, I think that would be preferable, rather than making the code less flexible. Otherwise there are various places where comments need updating.

[TaWeiTu]

In D96650#2561834, @fhahn wrote:

The main reason for this refactor is to help passing ArrayRef to function processLoop() (remove the need to swap elements in loop list).

Could you elaborate why this is needed to pass ArrayRef? I think stylistically passing the outer-loop Id explicitly makes the functions a bit clearer and the functions currently are written in a way that does not rely on passing adjacent loops. If possible to keep the parameters, I think that would be preferable, rather than making the code less flexible. Otherwise there are various places where comments need updating.

Oh, I see. I think my wording is not accurate enough.
The place that makes passing ArrayRef possible is where InnerLoop and OuterLoop are both explicitly passed to function processLoop, and I just find it a bit redundant to pass all InnerLoop, OuterLoop, InnerLoopId and OuterLoopId while replacing LoopList. But if the pass will potentially interchange non-adjacent loops in the future, keeping the parameters seems reasonable.

[TaWeiTu]

Address comment: Keep parameter OuterLoopId. Update the title and summary of this patch as well.

[fhahn]

LGTM. thanks!

llvm/lib/Transforms/Scalar/LoopInterchange.cpp

#endif
     if (!LoopNestExit) {
       LLVM_DEBUG(dbgs() << "OuterMostLoop needs an unique exit block");
       return false;
     }
 
     unsigned SelecLoopId = selectLoopForInterchange(LoopList);
     // Move the selected loop outwards to the best possible position.
     for (unsigned i = SelecLoopId; i > 0; i--) {
-      bool Interchanged =
-          processLoop(LoopList, i, i - 1, LoopNestExit, DependencyMatrix);
+      bool Interchanged = processLoop(LoopList[i], LoopList[i - 1], i, i - 1,
+                                      LoopNestExit, DependencyMatrix);
       if (!Interchanged)
         return Changed;
       // Loops interchanged reflect the same in LoopList
       std::swap(LoopList[i - 1], LoopList[i]);
 
       // Update the DependencyMatrix
       interChangeDependencies(DependencyMatrix, i, i - 1);
 #ifdef DUMP_DEP_MATRICIES
       LLVM_DEBUG(dbgs() << "Dependence after interchange\n");
       printDepMatrix(DependencyMatrix);
 #endif
       Changed |= Interchanged;
     }
     return Changed;
   }
 
-  bool processLoop(LoopVector LoopList, unsigned InnerLoopId,
+  bool processLoop(Loop *InnerLoop, Loop *OuterLoop, unsigned InnerLoopId,
                    unsigned OuterLoopId, BasicBlock *LoopNestExit,
                    std::vector<std::vector<char>> &DependencyMatrix) {
-    LLVM_DEBUG(dbgs() << "Processing Inner Loop Id = " << InnerLoopId
+    LLVM_DEBUG(dbgs() << "Processing InnerLoopId = " << InnerLoopId
                       << " and OuterLoopId = " << OuterLoopId << "\n");
-    Loop *InnerLoop = LoopList[InnerLoopId];
-    Loop *OuterLoop = LoopList[OuterLoopId];
 
     LoopInterchangeLegality LIL(OuterLoop, InnerLoop, SE, ORE);
     if (!LIL.canInterchangeLoops(InnerLoopId, OuterLoopId, DependencyMatrix)) {
       LLVM_DEBUG(dbgs() << "Not interchanging loops. Cannot prove legality.\n");
       return false;
     }
     LLVM_DEBUG(dbgs() << "Loops are legal to interchange\n");
     LoopInterchangeProfitability LIP(OuterLoop, InnerLoop, SE, ORE);

llvm/test/Transforms/LoopInterchange/interchange-flow-dep-outer.ll

 ;;  for (int k = 0; k < 100; ++k) {
 ;;    T[k] = fn1();
 ;;    for (int i = 0; i < 1000; ++i)
 ;;      for(int j = 1; j < 1000; ++j)
 ;;        Arr[j][i] = Arr[j][i]+k;
 ;;    fn2(T[k]);
 ;;  }
 
-; CHECK: Processing Inner Loop Id = 2 and OuterLoopId = 1
+; CHECK: Processing InnerLoopId = 2 and OuterLoopId = 1
 ; CHECK: Loops interchanged.
 
-; CHECK: Processing Inner Loop Id = 1 and OuterLoopId = 0
+; CHECK: Processing InnerLoopId = 1 and OuterLoopId = 0
 ; CHECK: Not interchanging loops. Cannot prove legality.
 
 @T = internal global [100 x double] zeroinitializer, align 4
 @Arr = internal global [1000 x [1000 x i32]] zeroinitializer, align 4
 
 define void @interchange_09(i32 %k) {
 entry:
   br label %for.body

llvm/test/Transforms/LoopInterchange/not-interchanged-loop-nest-3.ll

 ; REQUIRES: asserts
 ; RUN: opt < %s -basic-aa -loop-interchange -verify-dom-info -verify-loop-info \
 ; RUN:     -S -debug 2>&1 | FileCheck %s
 
 target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-unknown-linux-gnu"
 
 @D = common global [100 x [100 x [100 x i32]]] zeroinitializer
 
 ;; Test for interchange in loop nest greater than 2.
 ;;  for(int i=0;i<100;i++)
 ;;    for(int j=0;j<100;j++)
 ;;      for(int k=0;k<100;k++)
 ;;        D[i][k][j] = D[i][k][j]+t;
 
-; CHECK: Processing Inner Loop Id = 2 and OuterLoopId = 1
+; CHECK: Processing InnerLoopId = 2 and OuterLoopId = 1
 ; CHECK: Loops interchanged.
 
-; CHECK: Processing Inner Loop Id = 1 and OuterLoopId = 0
+; CHECK: Processing InnerLoopId = 1 and OuterLoopId = 0
 ; CHECK: Interchanging loops not profitable.
 
 define void @interchange_08(i32 %t){
 entry:
   br label %for.cond1.preheader
 
 for.cond1.preheader:                              ; preds = %for.inc15, %entry
   %i.028 = phi i32 [ 0, %entry ], [ %inc16, %for.inc15 ]