Skip to content

Conversation

davemgreen
Copy link
Collaborator

In certain situations, especially with zero phi operands propagated after tail duplications, we can end up with CBZ/CBNZ/TBZ/TBNZ with a zero register. It only happens late in the pipeline. This patch adds a basic simplifyInstruction to fold them away to either a direct branch or removing the instruction entirely.

It needs some fixups, but seems to work.

In certain situations, especially with zero phi operands propagated after tail
duplications, we can end up with CBZ/CBNZ/TBZ/TBNZ with a zero register. It
only happens late in the pipeline. This patch adds a basic simplifyInstruction
to fold them away to either a direct branch or removing the instruction
entirely.

It needs some fixups, but seems to work.
Copy link

github-actions bot commented Oct 1, 2025

⚠️ C/C++ code formatter, clang-format found issues in your code. ⚠️

You can test this locally with the following command:
git-clang-format --diff origin/main HEAD --extensions h,cpp -- llvm/include/llvm/CodeGen/TargetInstrInfo.h llvm/lib/CodeGen/MachineCopyPropagation.cpp llvm/lib/CodeGen/ShrinkWrap.cpp llvm/lib/Target/AArch64/AArch64InstrInfo.cpp llvm/lib/Target/AArch64/AArch64InstrInfo.h llvm/lib/Target/RISCV/RISCVInstrInfo.cpp llvm/lib/Target/RISCV/RISCVInstrInfo.h

⚠️
The reproduction instructions above might return results for more than one PR
in a stack if you are using a stacked PR workflow. You can limit the results by
changing origin/main to the base branch/commit you want to compare against.
⚠️

View the diff from clang-format here.
diff --git a/llvm/lib/Target/AArch64/AArch64InstrInfo.cpp b/llvm/lib/Target/AArch64/AArch64InstrInfo.cpp
index 99a182012..4cdd370e2 100644
--- a/llvm/lib/Target/AArch64/AArch64InstrInfo.cpp
+++ b/llvm/lib/Target/AArch64/AArch64InstrInfo.cpp
@@ -704,7 +704,7 @@ bool AArch64InstrInfo::simplifyInstruction(MachineInstr &MI,
       for (auto *S : Succs)
         if (S != Target)
           MBB->removeSuccessor(S);
-      SmallVector<MachineInstr*> DeadInstrs;
+      SmallVector<MachineInstr *> DeadInstrs;
       for (auto It = MI.getIterator(); It != MBB->end(); ++It)
         DeadInstrs.push_back(&*It);
       BuildMI(MBB, MI.getDebugLoc(), get(AArch64::B)).addMBB(Target);

@dtellenbach
Copy link
Member

Would it make sense to also check for cb.{eq,ne} {x,w}zr, #0? Not sure if we'd ever codegen that though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants