[PATCH] D116273: [AMDGPU] Iterate LoweredEndCf in the reverse order

Mon Jan 3 04:21:46 PST 2022

cdevadas added a comment.

In D116273#3214831 <https://reviews.llvm.org/D116273#3214831>, @foad wrote:

> What was the effect of inserting multiple branch instructions? Did it fail MIR verification?

No, MIR verifier won't flag it as an error.
It becomes an assertion later at https://github.com/llvm/llvm-project/blob/main/llvm/lib/Target/AMDGPU/SILateBranchLowering.cpp#L149

In the case of multiple unconditional branches, most targets (in their analyzeBranch implementation) remove all but the first one (Arch64, ARM, and a few more). 
But we don't do any such thing in the analyzeBranch. Not sure if there is any specific reason not to do so for AMDGPU.

================
Comment at: llvm/lib/Target/AMDGPU/SILowerControlFlow.cpp:585

-  for (MachineInstr *MI : LoweredEndCf) {
+  for (MachineInstr *MI : llvm::reverse(LoweredEndCf)) {
     MachineBasicBlock &MBB = *MI->getParent();
----------------
foad wrote:
> There's a "using namespace llvm" at the top of this file.
Thanks for pointing it out. Will simplify it.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116273