[PATCH] D112402: [SCEV][NFC] Verify intergity of SCEVUsers

[Philip Reames via Phabricator via llvm-commits]

reames added a comment.

In D112402#3083147 <https://reviews.llvm.org/D112402#3083147>, @mkazantsev wrote:

> Not 100% sure it should be UniqueSCEVs to iterate on.

I think it's the right one.  This is a property which should hold for all SCEVs.  We'll miss stale unknowns (since they're removed from that set), but that's okay because they have no operands.

> besides, we don't verify obsolete dangling users so far (at least not before we start dropping them)

I have no idea what you mean by this.  Expand?

I'd LGTM this except for open question above.  I'm concerned I'm missing something.



================
Comment at: llvm/lib/Analysis/ScalarEvolution.cpp:12905
+    SmallPtrSet<const SCEV *, 4> Ops;
+    if (const auto *NS = dyn_cast<SCEVNAryExpr>(&S))
+      Ops.insert(NS->op_begin(), NS->op_end());
----------------
This code exists in at least one other place (getDefiningScopeBound).  I'm fine with this landing as is, but we should probably start thinking about how to abstract user and operand walks.  


================
Comment at: llvm/lib/Analysis/ScalarEvolution.cpp:12918
+      auto It = SCEVUsers.find(Op);
+      if (It == SCEVUsers.end() || !It->second.count(&S)) {
+        dbgs() << "Use of operand  " << *Op << " by user " << S
----------------
Minor: Can you make the normal case an early continue to reduce nesting?


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

https://reviews.llvm.org/D112402