[PATCH] D127178: Fix FunctionPropertiesAnalysis updating callsite in 1-BB loop
Kazu Hirata via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Tue Jun 7 14:52:05 PDT 2022
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
More information about the llvm-commits
mailing list