[PATCH] D83094: Analysis: Add a GenericCycleInfo analysis

[Nicolai Hähnle via Phabricator via llvm-commits]

nhaehnle marked an inline comment as done.
nhaehnle added inline comments.


================
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;
----------------
arsenm wrote:
> 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
I'm going down the error message path. It is slightly more convenient -- the error messages probably aren't the most helpful, but at least they are distinct so you can always put a breakpoint there if you want to explore further.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83094