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

Wed Oct 12 11:26:28 PDT 2022

efriedma added inline comments.

================
Comment at: llvm/lib/CodeGen/BranchRelaxation.cpp:508-510
+    } else if (FailToAnalyze && (!PrevBB->back().isBarrier() ||
+                                 TII->isPredicated(PrevBB->back()))) {
+      InsertUncondBranch(PrevBB, DestBB);
----------------
arsenm wrote:
> efriedma wrote:
> > gandhi21299 wrote:
> > > arsenm wrote:
> > > > Is this case covered in the test?
> > > I tried out various branching opcodes out there on AMDGPU, none of them satisfied the condition.
> > I'd prefer to just check `else if (FailToAnalyze && PrevBB->isSuccessor(DestBB))`, rather than try to figure out the specific properties of the terminator of PrevBB.
> I'm not a fan of putting untested code in to satisfy theoretical conditions. If there's a case we don't know how to handle, I would just put a report_fatal_error in for when a target that does needs this runs into it
You could end up in this situation with a predicated "ret" instruction on 32-bit ARM, I think, but we don't use BranchRelaxation there.  Not sure if there any any unanalyzable fallthrough terminators on any of the targets that currently use BranchRelaxation; if there aren't, I guess a report_fatal_error is sufficient.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134557