[PATCH] D69823: [IR] PR27065: Added tests for BasicBlock::removePredecessor.

Tue Nov 5 05:05:38 PST 2019

lebedev.ri added a reviewer: spatel.
lebedev.ri added inline comments.

================
Comment at: llvm/test/Transforms/Inline/clone-function-no-assert-on-phis.ll:8-10
+; [[TMP81_I:%.*]] = getelementptr inbounds i8, i8* [[TMP81_I]], i64 1
+; It happens to work, because the code is dead and
+; verifier doesn't complain.
----------------
dendibakh wrote:
> lebedev.ri wrote:
> > Can that happen for non-dead code?
> > That is perfectly okay for dead, unreachable code.
> I think it doesn't happen for non-dead code. Otherwise, verifier would tell us about that.
> Regarding BasicBlock::removePredecessor behavior: this is okay to leave broken code in unreachable blocks until someone will try to link detached blocks again. And we never know what will happen after user called removePredecessor. In this sense BasicBlock::removePredecessor should not leave broken code.
Please can you be more specific?
Is there more complete test case that demonstrates the issues this causes?

https://bugs.llvm.org/show_bug.cgi?id=27065
(which is no longer reproducible since that pass is gone)
reads to me as classical "pass gets overwhelmed by
the wonders of unreachable IR", which is solved by
not handling unreachable code, which also is faster:
rL372339 rL370911 rL355337 etc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69823