[PATCH] D127178: Fix FunctionPropertiesAnalysis updating callsite in 1-BB loop

[Kazu Hirata via Phabricator via llvm-commits]

kazu accepted this revision.
kazu added a comment.
This revision is now accepted and ready to land.

Thank you for the fix.  LGTM modulo some rewording of the comment.



================
Comment at: llvm/lib/Analysis/FunctionPropertiesAnalysis.cpp:138-142
+  // Exclude the CallSiteBB, if it happens to be its own successor (1-BB loop).
+  // We are only interested in BBs the graph moves past the callsite BB to
+  // define the frontier past which we don't want to re-process BBs. Including
+  // the callsite BB in this case would stop prematurely the traversal in
+  // finish().
----------------
I understand the motivation and the fix, but I am wondering if we can make the comment a little easier to understand.

```
// Exclude the CallSiteBB.  finish() traverses the resulting CFG from CallSiteBB                                                                  
// up to and including Successors.  Including the CallSiteBB in Successors would                                                                  
// prematurely stop the traversal when it happens to be its own successor. 
```

Nit: "would prematurely stop the traversal" sounds more natural than "would stop prematurely the traversal".



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127178