[PATCH] D127353: FunctionPropertiesAnalysis: handle callsite BBs that lose edges
Mircea Trofin via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Thu Jun 9 19:20:34 PDT 2022
mtrofin marked an inline comment as done.
mtrofin added inline comments.
================
Comment at: llvm/lib/Analysis/FunctionPropertiesAnalysis.cpp:195
+ for (const auto *Succ : Successors) {
+ if (!ReIncluded.contains(Succ) && !pred_empty(Succ)) {
+ FPI.reIncludeBB(*Succ, LI);
----------------
kazu wrote:
> I may be being paranoid here, but is `pred_empty(Succ)` a correct condition here?
>
> If `Succ` doesn't have any incoming edge, we can certainly disregard it. Now, if `Succ` has at least one incoming edge, could we have a corner case like this?
>
> - `SuccA` has no incoming edge after inlining, and
> - `SuccB` has an incoming edge from `SuccA` after inlining with no other incoming edge.
>
> In this case, neither `SuccA` nor `SuccB` is reachable from the entry basic block, but your algorithm would end up re-including `SuccB`.
>
> If we know for sure that this kind of thing never happens, I wonder if we can put an assert somewhere. For example, each successor has exactly one incoming edge prior to inlining because of exception semantics or something, we may be able to assert that as we populate `Successors`.
Good point - thanks - and we should be probably just using the dominator tree to see if a BB is reachable from the entry point. I need D127467 landed first, as this fix then depends on that.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D127353/new/
https://reviews.llvm.org/D127353
More information about the llvm-commits
mailing list