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

[Whitney Tsang via Phabricator via llvm-commits]

Whitney added inline comments.


================
Comment at: llvm/include/llvm/Analysis/LoopNestAnalysis.h:142
 
+  BasicBlock *getHeader() const { return Loops.front()->getHeader(); }
+
----------------
I am not sure it is clear that getHeader of a loop nest is the outermost loop header. 
Looks like you added it for getting the loopnest function?
`Function &F = *LN.getHeader()->getParent();`

maybe adding a getParent() is more appropriate? 


================
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
----------------
can we keep the const?


================
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);
----------------
why is it safe to remove 
```
      // Loops interchanged reflect the same in LoopList
      std::swap(LoopList[i - 1], LoopList[i]);
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96644