[PATCH] D74691: [Attributor] Detect possibly unbounded cycles in functions

[Stefanos Baziotis via Phabricator via llvm-commits]

baziotis added a comment.

In D74691#1914776 <https://reviews.llvm.org/D74691#1914776>, @omarahmed wrote:

> > Remove the TODO.
> >  Then please see the comment above.
>
> sry , I don't understand what u mean by the comment above , does u mean your comment to uenoko ?


No sorry, I meant this:

  Here, you want something clearer to what the function does. I'd say, you can go with:
  "Helper function that checks whether a function has any cycle which we don't know is bounded.
  Loops with maximum trip count are considered bounded, any other cycle not."
  We may have to revise this further but it seems a good start.



================
Comment at: llvm/lib/Transforms/IPO/Attributor.cpp:2429
+  // unbounded cycle.
+  // We use scc_iterator which uses Tarjan algorithm to find all the maximal
+  // SCCs. Those are SCCs which are as big as they can get. i.e. in which you
----------------
uenoku wrote:
> baziotis wrote:
> > uenoku wrote:
> > > nit: Normally, the word "strongly connected components" contains its maximality. So I think it's more concise to omit the description for maximality.
> > I'm not sure there's a definitive answer here. It depends on the definition. If we use the "reachability" definition, then it's not implied. An SCC can contain an SCC. But if we talk about plural, i.e. SCCs of a graph, the definition contains the term partition. Which implies maximality. I've seen both in books and I believe it's a long discussion in which we don't want to get the reader to. The fact that I didn't remember that `scc_iterators` finds maximal SCCs led to 3-4 days at least of working on a wrong solution (in this patch actually).
> Ok, I'm fine with either way :)
Cool :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74691