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

[omar ahmed via Phabricator via llvm-commits]

omarahmed added a comment.

In D74691#1907179 <https://reviews.llvm.org/D74691#1907179>, @baziotis wrote:

> In D74691#1906930 <https://reviews.llvm.org/D74691#1906930>, @omarahmed wrote:
>
> > [Attributor]Detect possibly unbounded cycles in functions
> >
> > so the new approach detects cycles by using a recursive function that dfs on the CFG
>
>
> Note that this algorithm detects whether there is //a// cycle in the function (correctl). But now
>  note that if you find a cycle, you may want to skip it. In that case, it is wrong.
>  Specifically, because of the `Processed`, say you find a cycle A and you skip it
>  because it is a loop with max trip count (how do you do that reliably is another story).
>  There can be a cycle B (in which some nodes of A are contained) that won't be found.
>  That is a false negative (i.e. we'll say the function is `willreturn` but it might
>  not be). I think this is a problem.
>  I'll have to think if that can happen in the CFG, but from the graph theory perspective,
>  it definitely can.
>
> > This new approach also saves the processed nodes so not compute them two times thus enhancing the complexity of the algorithm to be O(N+E).
> >  The approach also doesn't visit statically unreachable blocks so that also enhances the complexity.
>
> Those 2 were true for the previous algorithm as well. If I'm not mistaken, we're doing strictly more work than the previous algorithm (although
>  the facts you wrote are correct).


>From what i understaand is that when we have a case like that https://i.imgur.com/cvAjcMa.png and when cycle 1-2-3 have amax trip count and we visit them first we have a problem
but i think it won't make a problem since 2 and 3 which are in the bigger cycle must have edges in the cycle in here is the edge of 3 to 4 so 3 and 2 will not be marked processed until they completly finish the 3 node successors so it will complete in the bigger cycle and detect the cycle . I think marking processed at the end will make us escape from a problem like that


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74691