[PATCH] D97847: Use LoopNest as the primary object on which LoopInterchange operates

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

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


================
Comment at: llvm/include/llvm/Analysis/LoopNestAnalysis.h:153
+  /// of the outer loop.
+  void interchangeNestedLoops(unsigned OuterLoopId, unsigned InnerLoopId) {
+    assert((Loops[InnerLoopId]->getParentLoop() == Loops[OuterLoopId] &&
----------------
Whitney wrote:
> TaWeiTu wrote:
> > Whitney wrote:
> > > TaWeiTu wrote:
> > > > Whitney wrote:
> > > > > Thinking we should have a more generic set function, like setLoops?
> > > > Agreed. But I also think having `interchangeNestedLoops` here is good in the sense that we can do the `assert`s below.
> > > > Should I add `setLoop` in this patch?
> > > Can we have a generic set function with a generic assert (guarded by expensive checks macro)?
> > > like assert Loops[0] is the outermost loop, there are no other loops in between Loops[i] and Loops[i+1], Loops[Loops.size()-1] is the innermost loop, etc...
> > > 
> > > then we can have `interchangeNestedLoops` in interchange pass?
> > OK. But I think having a separate patch for that is more appropriate because this patch is primarily about removing the usage of `LoopList` and synchronizing the API between legacy and new pass manager.
> > Can I leave `interchangeNestedLoops` here for now and move it back to `LoopInterchange` after/in the patch of `setLoops`?
> ok, can you please add a TODO comment?
Thanks! Added TODO.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97847