[PATCH] D130891: [Docs] Improve cycle and closed path definitions

[Jannik Silvanus via Phabricator via llvm-commits]

jsilvanus added inline comments.


================
Comment at: llvm/docs/CycleTerminology.rst:28
+   with respect to this particular DFS. The header is always an entry node.
+4. In any depth-first search starting from the entry, the set of cycles
+   found in the CFG is the same. These are the *top-level cycles*
----------------
foad wrote:
> jsilvanus wrote:
> > foad wrote:
> > > Would it make sense to swap 3 and 4, so you can define the "top-level cycles" without even mentioning DFS?
> > You mean one could define "top-level cycles" without mentioning a particular DFS choice?
> > 
> > The current order has the advantage that entries and headers are adjacent, resembling their close relationship.
> > But I do not have a strong opinion here.
> > You mean one could define "top-level cycles" without mentioning a particular DFS choice?
> 
> Yes. The sentence "In any depth-first search starting from the entry, the set of cycles found in the CFG is the same" is really strange. Why would you even mention DFS here??? It should be clear from (1) that cycles have nothing to do with any DFS ordering. It's like saying "the set of cycles found in the CFG is the same regardless of whether Denmark and Canada share a land border" :)
> 
> I guess the only reason that (4) mentions a DFS is that (3) has already mentioned DFS. That's why I suggested reordering them. But perhaps we could keep the order the same and still remove DFS from (4)?
Ah, now I understand, and I think you are right.

The current wording probably originates from something like "The set of top-level cycles (i.e. cycles in the original graph) can be found by DFS. In particular, the set of cycles found this way does not depend on the DFS choice."

What about moving 4 to before 2 (to keep 2 and 3 together), and rephrasing to:

The set of cycles in the function CFG itself (and not a nontrivial subgraph of it) are called *top-level cycles*.
The top-level cycles can be found using a DFS from the entry node.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130891