[PATCH] D83094: Analysis: Add a GenericCycleInfo analysis

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added inline comments.


================
Comment at: llvm/lib/Analysis/GenericCycleInfo.cpp:420
+/// or that the right set of cycles in the CFG were found.
+void GenericCycleInfoBase::validateTree() const {
+  DenseSet<CfgBlockRef> blocks;
----------------
If this is going to be assert-based, might as well disable the whole function ifndef NDEBUG?


================
Comment at: llvm/lib/Analysis/GenericCycleInfo.cpp:427
+/// or that the right set of cycles in the CFG were found.
+void GenericCycleInfoBase::validateTree() const {
+  DenseSet<CfgBlockRef> blocks;
----------------
nhaehnle wrote:
> arsenm wrote:
> > I think it would be more helpful to have this return a bool for fail/pass, and not directly assert. The assert conditions could print more about why it's not valid (although there so many asserts, this might be annoying)
> It's not clear to me why the fail/pass return would be more helpful?
I sometimes have called these type of verifiers in the middle of passes in gdb but it's not that important


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83094