[PATCH] D82269: [TRE][NFC] Refactor Basic Block Processing

Tue Jun 23 06:53:46 PDT 2020

laytonio marked an inline comment as done.
laytonio added a comment.

I'll wait and rebase after D82085 <https://reviews.llvm.org/D82085>.

================
Comment at: llvm/lib/Transforms/Scalar/TailRecursionElimination.cpp:774
+    // eliminateCall will attempt to remove.
+    if (!Succ->hasAddressTaken() && pred_begin(Succ) == pred_end(Succ))
+      DTU.deleteBB(Succ);
----------------
efriedma wrote:
> I'm pretty sure the address-taken check here is wrong, and this doesn't interact correctly with unreachable code.  Probably we should teach eliminateCall to RAUW away dead uses instead, and then let some other code call removeUnreachableBlocks to do the cleanup properly.
> 
> I guess this isn't new code, though.
I don't think there is anything we could use to RAUW that would be semantic preserving, not that it matters much I guess since this block is now dead. I looked at the other uses of FoldReturnIntoUncondBranch (there are only 2) to see how other passes handle the possibility of creating a dead block. SimplifyCFG just checks the predecessors and deletes the block. CodeGenPrepare checks the predecessors and the address-taken and deletes the block. The BasicBlock destructor seems to correctly handle the case where a dead block has its address taken, and even a nice comment explaining how you could end up in that situation. Therefore, I think we are safe to remove the address-taken check and just delete the block if it has no predecessors.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82269