[PATCH] D96644: [NFC] Refactor LoopInterchange into a loop-nest pass

[Ta-Wei Tu via Phabricator via llvm-commits]

TaWeiTu marked 2 inline comments as done.
TaWeiTu added inline comments.


================
Comment at: llvm/lib/Transforms/Scalar/LoopInterchange.cpp:479
 
-  unsigned selectLoopForInterchange(const LoopVector &LoopList) {
+  unsigned selectLoopForInterchange(ArrayRef<Loop *> LoopList) {
     // TODO: Add a better heuristic to select the loop to be interchanged based
----------------
Whitney wrote:
> can we keep the const?
I thought that `ArrayRef` is by definition a constant reference to the array. Am I missing something here?


================
Comment at: llvm/lib/Transforms/Scalar/LoopInterchange.cpp:532
         return Changed;
-      // Loops interchanged reflect the same in LoopList
-      std::swap(LoopList[i - 1], LoopList[i]);
-
       // Update the DependencyMatrix
       interChangeDependencies(DependencyMatrix, i, i - 1);
----------------
Whitney wrote:
> why is it safe to remove 
> ```
>       // Loops interchanged reflect the same in LoopList
>       std::swap(LoopList[i - 1], LoopList[i]);
> ```
Because previously the only place where `LoopList` is used is to retrieve the `InnerLoop` and `OuterLoop` to be interchanged by their indices in the array. After D96650, both loops are now explicitly passed to `processLoop` and thus maintaining the order of the loops in `LoopList` is irrelevant now.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D96644/new/

https://reviews.llvm.org/D96644