[llvm] [RISCV] Explicitly check for supported opcodes in optimizeCondBranch. NFC (PR #145622)
Craig Topper via llvm-commits
llvm-commits at lists.llvm.org
Wed Jun 25 21:12:02 PDT 2025
https://github.com/topperc updated https://github.com/llvm/llvm-project/pull/145622
>From cfb5abc625e975779cc583c507df8aa4075e0a1c Mon Sep 17 00:00:00 2001
From: Craig Topper <craig.topper at sifive.com>
Date: Tue, 24 Jun 2025 17:01:03 -0700
Subject: [PATCH 1/2] [RISCV] Explicitly check for supported opcodes in
optimizeCondBranch.
We don't support any of the immediate branches in this function yet
so explicitly exclude them rather than relying on isReg to return
false.
Remove use of AnalyzeBranch. It doesn't help us much. Part of the
code was already getting the operands directly and it just complicated
creating a new branch.
I also inlined the modifyBranch function so we could use addReg on
BuildMI.
Ultimately I want to try to fix that the RISCVCC enum has turned into
a proxy for opcode by having an entry for each vendor's branch
instructions. So I probably won't commit this until I figure out if
this helps move in that direction or if other changes are needed.
---
llvm/lib/Target/RISCV/RISCVInstrInfo.cpp | 79 ++++++++++++++----------
1 file changed, 45 insertions(+), 34 deletions(-)
diff --git a/llvm/lib/Target/RISCV/RISCVInstrInfo.cpp b/llvm/lib/Target/RISCV/RISCVInstrInfo.cpp
index 5711f0077b12d..9defcd5c85053 100644
--- a/llvm/lib/Target/RISCV/RISCVInstrInfo.cpp
+++ b/llvm/lib/Target/RISCV/RISCVInstrInfo.cpp
@@ -1379,40 +1379,50 @@ bool RISCVInstrInfo::isFromLoadImm(const MachineRegisterInfo &MRI,
}
bool RISCVInstrInfo::optimizeCondBranch(MachineInstr &MI) const {
+ bool IsSigned = false;
+ bool IsEquality = false;
+ switch (MI.getOpcode()) {
+ default:
+ return false;
+ case RISCV::BEQ:
+ case RISCV::BNE:
+ IsEquality = true;
+ break;
+ case RISCV::BGE:
+ case RISCV::BLT:
+ IsSigned = true;
+ break;
+ case RISCV::BGEU:
+ case RISCV::BLTU:
+ break;
+ }
+
MachineBasicBlock *MBB = MI.getParent();
MachineRegisterInfo &MRI = MBB->getParent()->getRegInfo();
- MachineBasicBlock *TBB, *FBB;
- SmallVector<MachineOperand, 3> Cond;
- if (analyzeBranch(*MBB, TBB, FBB, Cond, /*AllowModify=*/false))
- return false;
+ const MachineOperand &LHS = MI.getOperand(0);
+ const MachineOperand &RHS = MI.getOperand(1);
+ MachineBasicBlock *TBB = MI.getOperand(2).getMBB();
- RISCVCC::CondCode CC = static_cast<RISCVCC::CondCode>(Cond[0].getImm());
+ RISCVCC::CondCode CC = getCondFromBranchOpc(MI.getOpcode());
assert(CC != RISCVCC::COND_INVALID);
- auto modifyBranch = [&]() {
- // Build the new branch and remove the old one.
- BuildMI(*MBB, MI, MI.getDebugLoc(),
- getBrCond(static_cast<RISCVCC::CondCode>(Cond[0].getImm())))
- .add(Cond[1])
- .add(Cond[2])
- .addMBB(TBB);
- MI.eraseFromParent();
- };
-
// Canonicalize conditional branches which can be constant folded into
// beqz or bnez. We can't modify the CFG here.
int64_t C0, C1;
- if (isFromLoadImm(MRI, Cond[1], C0) && isFromLoadImm(MRI, Cond[2], C1)) {
- unsigned NewCC =
- evaluateCondBranch(CC, C0, C1) ? RISCVCC::COND_EQ : RISCVCC::COND_NE;
- Cond[0] = MachineOperand::CreateImm(NewCC);
- Cond[1] = Cond[2] = MachineOperand::CreateReg(RISCV::X0, /*isDef=*/false);
- modifyBranch();
+ if (isFromLoadImm(MRI, LHS, C0) && isFromLoadImm(MRI, RHS, C1)) {
+ unsigned NewOpc = evaluateCondBranch(CC, C0, C1) ? RISCV::BEQ : RISCV::BNE;
+ MachineOperand Zero = MachineOperand::CreateReg(RISCV::X0, /*isDef=*/false);
+ // Build the new branch and remove the old one.
+ BuildMI(*MBB, MI, MI.getDebugLoc(), get(NewOpc))
+ .add(Zero)
+ .add(Zero)
+ .addMBB(TBB);
+ MI.eraseFromParent();
return true;
}
- if (CC == RISCVCC::COND_EQ || CC == RISCVCC::COND_NE)
+ if (IsEquality)
return false;
// For two constants C0 and C1 from
@@ -1432,8 +1442,6 @@ bool RISCVInstrInfo::optimizeCondBranch(MachineInstr &MI) const {
//
// To make sure this optimization is really beneficial, we only
// optimize for cases where Y had only one use (i.e. only used by the branch).
- MachineOperand &LHS = MI.getOperand(0);
- MachineOperand &RHS = MI.getOperand(1);
// Try to find the register for constant Z; return
// invalid register otherwise.
auto searchConst = [&](int64_t C1) -> Register {
@@ -1449,23 +1457,25 @@ bool RISCVInstrInfo::optimizeCondBranch(MachineInstr &MI) const {
return Register();
};
+ unsigned NewOpc = RISCVCC::getBrCond(getOppositeBranchCondition(CC));
+
// Might be case 1.
// Don't change 0 to 1 since we can use x0.
// For unsigned cases changing -1U to 0 would be incorrect.
// The incorrect case for signed would be INT_MAX, but isFromLoadImm can't
// return that.
if (isFromLoadImm(MRI, LHS, C0) && C0 != 0 && LHS.getReg().isVirtual() &&
- MRI.hasOneUse(LHS.getReg()) &&
- (CC == RISCVCC::COND_GE || CC == RISCVCC::COND_LT || C0 != -1)) {
+ MRI.hasOneUse(LHS.getReg()) && (IsSigned || C0 != -1)) {
assert(isInt<12>(C0) && "Unexpected immediate");
if (Register RegZ = searchConst(C0 + 1)) {
- reverseBranchCondition(Cond);
- Cond[1] = MachineOperand::CreateReg(RHS.getReg(), /*isDef=*/false);
- Cond[2] = MachineOperand::CreateReg(RegZ, /*isDef=*/false);
+ BuildMI(*MBB, MI, MI.getDebugLoc(), get(NewOpc))
+ .add(RHS)
+ .addReg(RegZ)
+ .addMBB(TBB);
// We might extend the live range of Z, clear its kill flag to
// account for this.
MRI.clearKillFlags(RegZ);
- modifyBranch();
+ MI.eraseFromParent();
return true;
}
}
@@ -1479,13 +1489,14 @@ bool RISCVInstrInfo::optimizeCondBranch(MachineInstr &MI) const {
MRI.hasOneUse(RHS.getReg())) {
assert(isInt<12>(C0) && "Unexpected immediate");
if (Register RegZ = searchConst(C0 - 1)) {
- reverseBranchCondition(Cond);
- Cond[1] = MachineOperand::CreateReg(RegZ, /*isDef=*/false);
- Cond[2] = MachineOperand::CreateReg(LHS.getReg(), /*isDef=*/false);
+ BuildMI(*MBB, MI, MI.getDebugLoc(), get(NewOpc))
+ .addReg(RegZ)
+ .add(LHS)
+ .addMBB(TBB);
// We might extend the live range of Z, clear its kill flag to
// account for this.
MRI.clearKillFlags(RegZ);
- modifyBranch();
+ MI.eraseFromParent();
return true;
}
}
>From 817d4976ec8f54a0f8826ef3cfdd8e9fcabab01d Mon Sep 17 00:00:00 2001
From: Craig Topper <craig.topper at sifive.com>
Date: Wed, 25 Jun 2025 21:11:34 -0700
Subject: [PATCH 2/2] fixup! Use addReg(RISCV::X0)
---
llvm/lib/Target/RISCV/RISCVInstrInfo.cpp | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/llvm/lib/Target/RISCV/RISCVInstrInfo.cpp b/llvm/lib/Target/RISCV/RISCVInstrInfo.cpp
index 9defcd5c85053..38a6c043b235d 100644
--- a/llvm/lib/Target/RISCV/RISCVInstrInfo.cpp
+++ b/llvm/lib/Target/RISCV/RISCVInstrInfo.cpp
@@ -1412,11 +1412,10 @@ bool RISCVInstrInfo::optimizeCondBranch(MachineInstr &MI) const {
int64_t C0, C1;
if (isFromLoadImm(MRI, LHS, C0) && isFromLoadImm(MRI, RHS, C1)) {
unsigned NewOpc = evaluateCondBranch(CC, C0, C1) ? RISCV::BEQ : RISCV::BNE;
- MachineOperand Zero = MachineOperand::CreateReg(RISCV::X0, /*isDef=*/false);
// Build the new branch and remove the old one.
BuildMI(*MBB, MI, MI.getDebugLoc(), get(NewOpc))
- .add(Zero)
- .add(Zero)
+ .addReg(RISCV::X0)
+ .addReg(RISCV::X0)
.addMBB(TBB);
MI.eraseFromParent();
return true;
More information about the llvm-commits
mailing list