[PATCH] D134557: [BranchRelaxation] Fall through only if block has no terminators

[Anshil Gandhi via Phabricator via llvm-commits]

gandhi21299 added inline comments.


================
Comment at: llvm/lib/CodeGen/BranchRelaxation.cpp:485
     MachineBasicBlock *PrevBB = &*std::prev(DestBB->getIterator());
-    if (auto *FT = PrevBB->getFallThrough()) {
-      assert(FT == DestBB);
-      TII->insertUnconditionalBranch(*PrevBB, FT, DebugLoc());
-      // Recalculate the block size.
-      BlockInfo[PrevBB->getNumber()].Size = computeBlockSize(*PrevBB);
+    if (PrevBB->terminators().empty()) {
+      if (auto *FT = PrevBB->getFallThrough()) {
----------------
efriedma wrote:
> gandhi21299 wrote:
> > arsenm wrote:
> > > I'd expect for this to be covered by getFallThrough check for false. Did something weird happen with an unanalyzable branch?
> > We found a case where fixupUnconditionalBranch() appends a s_branch X to a block with terminators s_cbranch X, s_branch Y.
> There's a comment in getFallThrough: "If there is some explicit branch to the fallthrough block, it can obviously reach, even though the branch should get folded to fall through implicitly."  Maybe that bit of code is what's confusing the logic here?
> 
> (I'm not sure why that logic exists...)
Yea there is no check for the "explicit branch" or any form of branch at all.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134557