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

[Kazu Hirata via Phabricator via llvm-commits]

kazu 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);
----------------
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`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127353