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

Ta-Wei Tu via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Mar 8 06:51:18 PST 2021


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:
> > > 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`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97847



More information about the llvm-commits mailing list