[llvm-branch-commits] [llvm] [polly] [Support] Use block numbers for LoopInfo BBMap (PR #103400)

[Alexis Engelke via llvm-branch-commits]

================
@@ -756,14 +762,33 @@ void LoopInfoBase<BlockT, LoopT>::verify(
 
 // Verify that blocks are mapped to valid loops.
 #ifndef NDEBUG
-  for (auto &Entry : BBMap) {
-    const BlockT *BB = Entry.first;
-    LoopT *L = Entry.second;
-    assert(Loops.count(L) && "orphaned loop");
-    assert(L->contains(BB) && "orphaned block");
-    for (LoopT *ChildLoop : *L)
-      assert(!ChildLoop->contains(BB) &&
-             "BBMap should point to the innermost loop containing BB");
+  if constexpr (GraphHasNodeNumbers<const BlockT *>) {
+    for (auto It : enumerate(BBMap)) {
+      LoopT *L = It.value();
+      unsigned Number = It.index();
+      if (!L)
+        continue;
+      assert(Loops.count(L) && "orphaned loop");
+      // We have no way to map block numbers back to blocks, so find it.
+      auto BBIt = find_if(L->Blocks, [&Number](BlockT *BB) {
+        return GraphTraits<BlockT *>::getNumber(BB) == Number;
+      });
----------------
aengelke wrote:

We only need the block for contains(), so the loops could store a set of numbers instead of a set of block pointers. This is just very annoying to implement if we want to keep supporting IRs without numbers (MLIR is the problem here; Bolt doesn't expose the number but has one).

LoopInfo is asymptotically bad already, it is practically O(n^3) for deeply nested loops.. and uses O(n^2) storage in nesting depth (every level stores all blocks of the enclosed loops). It would be *much* easier to implement an efficient (both asymptotically and practically) LoopInfo if we gave up on updating (store loop-dfs and innermost loop of blocks, loop stores start/end index into block array and dfs numbers of loop forest). LoopInfo is doable in O(n log n).

https://github.com/llvm/llvm-project/pull/103400