[PATCH] D88166: [SCEV] Verify that all mapped SCEV AddRecs refer to valid loops.

[Florian Hahn via Phabricator via llvm-commits]

fhahn updated this revision to Diff 294932.
fhahn added a comment.



In D88166#2298419 <https://reviews.llvm.org/D88166#2298419>, @reames wrote:

> A cleaner way to handle this check would be to enumerate all loops in LoopInfo, and then check that the loop of every AR is in that set.  I the loop has already been deleted, dereferencing it to check the valid flags seems a bit suspect.

Sounds good, I updated the code here, as it seems clearer.

> Actually, why not just implement the isInvalid check on Loop that way?

LoopInfo manages the memory for each allocated loop and the memory is only really deleted when LI's memory is released. So the pointers to loops should stay valid as long as LI is alive, which allows for this quick check if the loop is valid. Not sure if there's a big benefit from changing that at the moment. What do you think?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88166

Files:
  llvm/lib/Analysis/ScalarEvolution.cpp


Index: llvm/lib/Analysis/ScalarEvolution.cpp
===================================================================
--- llvm/lib/Analysis/ScalarEvolution.cpp
+++ llvm/lib/Analysis/ScalarEvolution.cpp
@@ -12000,6 +12000,25 @@
       std::abort();
     }
   }
+
+  // Collect all valid loops currently in LoopInfo.
+  SmallPtrSet<Loop *, 32> ValidLoops;
+  SmallVector<Loop *, 32> Worklist(LI.begin(), LI.end());
+  while (!Worklist.empty()) {
+    Loop *L = Worklist.pop_back_val();
+    if (ValidLoops.contains(L))
+      continue;
+    ValidLoops.insert(L);
+    Worklist.append(L->begin(), L->end());
+  }
+  // Check for SCEV expressions referencing invalid/deleted loops.
+  for (auto &KV : ValueExprMap) {
+    auto *AR = dyn_cast<SCEVAddRecExpr>(KV.second);
+    if (!AR)
+      continue;
+    assert(ValidLoops.contains(AR->getLoop()) &&
+           "AddRec references invalid loop");
+  }
 }
 
 bool ScalarEvolution::invalidate(


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D88166.294932.patch
Type: text/x-patch
Size: 934 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200929/f6d2b55b/attachment.bin>