[PATCH] D114784: [SCEV] Track backedge taken count users (NFCI)

[Philip Reames via Phabricator via llvm-commits]

reames requested changes to this revision.
reames added inline comments.
This revision now requires changes to proceed.


================
Comment at: llvm/lib/Analysis/ScalarEvolution.cpp:7891
          "No point in having a non-constant max backedge taken count!");
-
-  SCEVRecordOperands RecordOperands(Operands);
----------------
Basic idea makes sense, but I really think the user tracking should be here.  Much easier to tell for sure we caught everything.

For one thing, you don't appear to be tracking the constantmax information in your patch, and I think we do need to add that.


================
Comment at: llvm/lib/Analysis/ScalarEvolution.cpp:12867
+      if (!isa<SCEVConstant>(ENT.ExactNotTaken)) {
+        auto UserIt = BECountUsers.find(ENT.ExactNotTaken);
+        assert(UserIt != BECountUsers.end());
----------------
Personally, I'd write this as:
assert BECountUsers.count(ENT.ExactNotTaken)
BECountUsers.erase(ENT.ExactNotTaken)

I often find the use of iterators confusing, and prefer to avoid them for simple asserts.  Why reason about potential invalidation when you don't have to?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114784