[PATCH] D84264: [SCCP] Directly remove non-feasible edges

[Eli Friedman via Phabricator via llvm-commits]

efriedma added inline comments.


================
Comment at: llvm/lib/Transforms/Scalar/SCCP.cpp:1834
+            isa<IndirectBrInst>(TI)) &&
+           "Terminator must be a br, switch or indirectbr");
+    BasicBlock *SingleSucc = *FeasibleSuccessors.begin();
----------------
Can we skip creating the unconditional branch if the terminator is already an unconditional branch?

Is it possible for this code to see other branches which inherently only have one successor, e.g. catchret?


================
Comment at: llvm/lib/Transforms/Scalar/SCCP.cpp:1836
+    BasicBlock *SingleSucc = *FeasibleSuccessors.begin();
+    BranchInst::Create(SingleSucc, BB);
+    TI->eraseFromParent();
----------------
nikic wrote:
> efriedma wrote:
> > Even if only one basic block is a possible successor, there could still be multiple edges, for example in a switch.  (This matters because you might need to update PHI nodes.)
> > 
> > ConstantFoldTerminator does some stuff with profile metadata; do we need something similar here?
> The profile metadata adjustment is only done when merging cases into the default destination. We don't need to adjust metadata if we convert into an unconditional branch.
> Even if only one basic block is a possible successor, there could still be multiple edges, for example in a switch. (This matters because you might need to update PHI nodes.)

Any response to this?


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

https://reviews.llvm.org/D84264