[PATCH] D69865: [IR] PR27065: Part2. Fix BasicBlock::removePredecessor to not break SSA form.

[Denis Bakhvalov via Phabricator via llvm-commits]

dendibakh added a comment.

I'm not arguing that passes should not look into unreachable code.

But in this exact case, BasicBlock::removePredecessor is pretty much general API, not limited to particular pass. We are not 100% sure that the broken code will be unreachable. 
Consider this:

  define i32 @f(i1 %cmp, i8 *%call) {
  entry:
    br i1 %cmp, label %phi.bb, label %ret.bb
  phi.bb:
    %ptr.1 = phi i8* [ %call, %entry],[ %inc.ptr1, %use.bb ]
    br label %use.bb
  use.bb:
    %inc.ptr1 = getelementptr inbounds i8, i8* %ptr.1, i64 1
    br i1 %cmp, label %ret.bb, label %phi.bb
  ret.bb:
    ret i32 0
  }

After doing:

  PhiBB->removePredecessor(EntryBB);

We have:

  define i32 @f(i1 %cmp, i8* %call) {
  entry:
    br i1 %cmp, label %phi.bb, label %ret.bb				<== edge entry->phi.bb will be removed later
  phi.bb:
    br label %use.bb
  use.bb:
    %inc.ptr1 = getelementptr inbounds i8, i8* %inc.ptr1, i64 1 		<== broken
    br i1 %cmp, label %ret.bb, label %phi.bb
  ret.bb:
    ret i32 0
  }

User can later decide to link phi.bb into some other place (outlining, for example). Can that happen? If yes, it is not unreachable code anymore.

To summarize my point: even if we know the code is dead and noone will look into it, I don't see benefits of allowing it to be broken. In this particular case, (I hope) it will require no additional changes, since passes do not look into unreachable code and do not care if it is valid or not.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69865