[PATCH] D127353: FunctionPropertiesAnalysis: handle callsite BBs that lose edges

[Mircea Trofin via Phabricator via llvm-commits]

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