Skip to content

[RISCV] Canonicalize foldable branch conditions in optimizeCondBranch #132988

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Mar 27, 2025

Conversation

preames
Copy link
Collaborator

@preames preames commented Mar 25, 2025

optimizeCondBranch isn't allowed to modify the CFG, but it can rewrite the branch condition freely. However, If we could fold a conditional branch to an unconditional one (aside from that restriction), we can also rewrite it into some canonical conditional branch instead.

Looking at the diffs, the only cases this catches in tree tests are cases where we could have constant folded during lowering from IR, but didn't. This is inspired by trying to salvage code from #131684 which might be useful. Given the test impact, it's of questionable merits. The main advantage over only the late cleanup pass is that it kills off the LIs for the constants early - which can help e.g. register allocation.

optimizeCondBranch isn't allowed to modify the CFG, but it can rewrite
the branch condition freely.  However, If we could fold a conditional
branch to an unconditional one (aside from that restriction), we can
also rewrite it into some canonical conditional branch instead.

Looking at the diffs, the only cases this catches in tree tests are
cases where we could have constant folded during lowering from IR,
but didn't.  This is inspired by trying to salvage code from llvm#131684
which might be useful.  Given the test impact, it's of questionable merits.
The main advantage over only the late cleanup pass is that it kills off the
LIs for the constants early - which can help e.g. register allocation.
@llvmbot
Copy link
Member

llvmbot commented Mar 25, 2025

@llvm/pr-subscribers-backend-risc-v

Author: Philip Reames (preames)

Changes

optimizeCondBranch isn't allowed to modify the CFG, but it can rewrite the branch condition freely. However, If we could fold a conditional branch to an unconditional one (aside from that restriction), we can also rewrite it into some canonical conditional branch instead.

Looking at the diffs, the only cases this catches in tree tests are cases where we could have constant folded during lowering from IR, but didn't. This is inspired by trying to salvage code from #131684 which might be useful. Given the test impact, it's of questionable merits. The main advantage over only the late cleanup pass is that it kills off the LIs for the constants early - which can help e.g. register allocation.


Patch is 25.14 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/132988.diff

6 Files Affected:

  • (modified) llvm/lib/Target/RISCV/RISCVInstrInfo.cpp (+68-34)
  • (modified) llvm/test/CodeGen/RISCV/bfloat-br-fcmp.ll (+4-8)
  • (modified) llvm/test/CodeGen/RISCV/double-br-fcmp.ll (+8-16)
  • (modified) llvm/test/CodeGen/RISCV/float-br-fcmp.ll (+8-16)
  • (modified) llvm/test/CodeGen/RISCV/half-br-fcmp.ll (+16-32)
  • (modified) llvm/test/CodeGen/RISCV/machine-sink-load-immediate.ll (+60-64)
diff --git a/llvm/lib/Target/RISCV/RISCVInstrInfo.cpp b/llvm/lib/Target/RISCV/RISCVInstrInfo.cpp
index 62f978d64fbb9..1d105806ecbd6 100644
--- a/llvm/lib/Target/RISCV/RISCVInstrInfo.cpp
+++ b/llvm/lib/Target/RISCV/RISCVInstrInfo.cpp
@@ -998,6 +998,26 @@ static RISCVCC::CondCode getCondFromBranchOpc(unsigned Opc) {
   }
 }
 
+
+static bool evaluateCondBranch(unsigned CC, int64_t C0, int64_t C1) {
+  switch (CC) {
+  default:
+    llvm_unreachable("Unexpected CC");
+  case RISCVCC::COND_EQ:
+    return C0 == C1;
+  case RISCVCC::COND_NE:
+    return C0 != C1;
+  case RISCVCC::COND_LT:
+    return C0 < C1;
+  case RISCVCC::COND_GE:
+    return C0 >= C1;
+  case RISCVCC::COND_LTU:
+    return (uint64_t)C0 < (uint64_t)C1;
+  case RISCVCC::COND_GEU:
+    return (uint64_t)C0 >= (uint64_t)C1;
+  }
+}
+
 // The contents of values added to Cond are not examined outside of
 // RISCVInstrInfo, giving us flexibility in what to push to it. For RISCV, we
 // push BranchOpcode, Reg1, Reg2.
@@ -1295,6 +1315,49 @@ bool RISCVInstrInfo::optimizeCondBranch(MachineInstr &MI) const {
   RISCVCC::CondCode CC = static_cast<RISCVCC::CondCode>(Cond[0].getImm());
   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();
+  };
+
+  // Right now we only care about LI (i.e. ADDI x0, imm)
+  auto isLoadImm = [](const MachineInstr *MI, int64_t &Imm) -> bool {
+    if (MI->getOpcode() == RISCV::ADDI && MI->getOperand(1).isReg() &&
+        MI->getOperand(1).getReg() == RISCV::X0) {
+      Imm = MI->getOperand(2).getImm();
+      return true;
+    }
+    return false;
+  };
+  // Either a load from immediate instruction or X0.
+  auto isFromLoadImm = [&](const MachineOperand &Op, int64_t &Imm) -> bool {
+    if (!Op.isReg())
+      return false;
+    Register Reg = Op.getReg();
+    if (Reg == RISCV::X0) {
+      Imm = 0;
+      return true;
+    }
+    return Reg.isVirtual() && isLoadImm(MRI.getVRegDef(Reg), Imm);
+  };
+
+  // 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(Cond[1], C0) && isFromLoadImm(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();
+    return true;
+  }
+
   if (CC == RISCVCC::COND_EQ || CC == RISCVCC::COND_NE)
     return false;
 
@@ -1315,24 +1378,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).
-
-  // Right now we only care about LI (i.e. ADDI x0, imm)
-  auto isLoadImm = [](const MachineInstr *MI, int64_t &Imm) -> bool {
-    if (MI->getOpcode() == RISCV::ADDI && MI->getOperand(1).isReg() &&
-        MI->getOperand(1).getReg() == RISCV::X0) {
-      Imm = MI->getOperand(2).getImm();
-      return true;
-    }
-    return false;
-  };
-  // Either a load from immediate instruction or X0.
-  auto isFromLoadImm = [&](const MachineOperand &Op, int64_t &Imm) -> bool {
-    if (!Op.isReg())
-      return false;
-    Register Reg = Op.getReg();
-    return Reg.isVirtual() && isLoadImm(MRI.getVRegDef(Reg), Imm);
-  };
-
   MachineOperand &LHS = MI.getOperand(0);
   MachineOperand &RHS = MI.getOperand(1);
   // Try to find the register for constant Z; return
@@ -1350,8 +1395,6 @@ bool RISCVInstrInfo::optimizeCondBranch(MachineInstr &MI) const {
     return Register();
   };
 
-  bool Modify = false;
-  int64_t C0;
   if (isFromLoadImm(LHS, C0) && MRI.hasOneUse(LHS.getReg())) {
     // Might be case 1.
     // Signed integer overflow is UB. (UINT64_MAX is bigger so we don't need
@@ -1364,7 +1407,8 @@ bool RISCVInstrInfo::optimizeCondBranch(MachineInstr &MI) const {
         // We might extend the live range of Z, clear its kill flag to
         // account for this.
         MRI.clearKillFlags(RegZ);
-        Modify = true;
+        modifyBranch();
+        return true;
       }
   } else if (isFromLoadImm(RHS, C0) && MRI.hasOneUse(RHS.getReg())) {
     // Might be case 2.
@@ -1378,22 +1422,12 @@ bool RISCVInstrInfo::optimizeCondBranch(MachineInstr &MI) const {
         // We might extend the live range of Z, clear its kill flag to
         // account for this.
         MRI.clearKillFlags(RegZ);
-        Modify = true;
+        modifyBranch();
+        return true;
       }
   }
 
-  if (!Modify)
-    return false;
-
-  // 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();
-
-  return true;
+  return false;
 }
 
 MachineBasicBlock *
diff --git a/llvm/test/CodeGen/RISCV/bfloat-br-fcmp.ll b/llvm/test/CodeGen/RISCV/bfloat-br-fcmp.ll
index 51ea8873d8c03..2e8cf35a294f6 100644
--- a/llvm/test/CodeGen/RISCV/bfloat-br-fcmp.ll
+++ b/llvm/test/CodeGen/RISCV/bfloat-br-fcmp.ll
@@ -11,8 +11,7 @@ declare bfloat @dummy(bfloat)
 define void @br_fcmp_false(bfloat %a, bfloat %b) nounwind {
 ; RV32IZFBFMIN-LABEL: br_fcmp_false:
 ; RV32IZFBFMIN:       # %bb.0:
-; RV32IZFBFMIN-NEXT:    li a0, 1
-; RV32IZFBFMIN-NEXT:    bnez a0, .LBB0_2
+; RV32IZFBFMIN-NEXT:    beqz zero, .LBB0_2
 ; RV32IZFBFMIN-NEXT:  # %bb.1: # %if.then
 ; RV32IZFBFMIN-NEXT:    ret
 ; RV32IZFBFMIN-NEXT:  .LBB0_2: # %if.else
@@ -22,8 +21,7 @@ define void @br_fcmp_false(bfloat %a, bfloat %b) nounwind {
 ;
 ; RV64IZFBFMIN-LABEL: br_fcmp_false:
 ; RV64IZFBFMIN:       # %bb.0:
-; RV64IZFBFMIN-NEXT:    li a0, 1
-; RV64IZFBFMIN-NEXT:    bnez a0, .LBB0_2
+; RV64IZFBFMIN-NEXT:    beqz zero, .LBB0_2
 ; RV64IZFBFMIN-NEXT:  # %bb.1: # %if.then
 ; RV64IZFBFMIN-NEXT:    ret
 ; RV64IZFBFMIN-NEXT:  .LBB0_2: # %if.else
@@ -583,8 +581,7 @@ if.then:
 define void @br_fcmp_true(bfloat %a, bfloat %b) nounwind {
 ; RV32IZFBFMIN-LABEL: br_fcmp_true:
 ; RV32IZFBFMIN:       # %bb.0:
-; RV32IZFBFMIN-NEXT:    li a0, 1
-; RV32IZFBFMIN-NEXT:    bnez a0, .LBB16_2
+; RV32IZFBFMIN-NEXT:    beqz zero, .LBB16_2
 ; RV32IZFBFMIN-NEXT:  # %bb.1: # %if.else
 ; RV32IZFBFMIN-NEXT:    ret
 ; RV32IZFBFMIN-NEXT:  .LBB16_2: # %if.then
@@ -594,8 +591,7 @@ define void @br_fcmp_true(bfloat %a, bfloat %b) nounwind {
 ;
 ; RV64IZFBFMIN-LABEL: br_fcmp_true:
 ; RV64IZFBFMIN:       # %bb.0:
-; RV64IZFBFMIN-NEXT:    li a0, 1
-; RV64IZFBFMIN-NEXT:    bnez a0, .LBB16_2
+; RV64IZFBFMIN-NEXT:    beqz zero, .LBB16_2
 ; RV64IZFBFMIN-NEXT:  # %bb.1: # %if.else
 ; RV64IZFBFMIN-NEXT:    ret
 ; RV64IZFBFMIN-NEXT:  .LBB16_2: # %if.then
diff --git a/llvm/test/CodeGen/RISCV/double-br-fcmp.ll b/llvm/test/CodeGen/RISCV/double-br-fcmp.ll
index 035228e73c707..42f1b1360a2d3 100644
--- a/llvm/test/CodeGen/RISCV/double-br-fcmp.ll
+++ b/llvm/test/CodeGen/RISCV/double-br-fcmp.ll
@@ -14,8 +14,7 @@ declare void @exit(i32)
 define void @br_fcmp_false(double %a, double %b) nounwind {
 ; RV32IFD-LABEL: br_fcmp_false:
 ; RV32IFD:       # %bb.0:
-; RV32IFD-NEXT:    li a0, 1
-; RV32IFD-NEXT:    bnez a0, .LBB0_2
+; RV32IFD-NEXT:    beqz zero, .LBB0_2
 ; RV32IFD-NEXT:  # %bb.1: # %if.then
 ; RV32IFD-NEXT:    ret
 ; RV32IFD-NEXT:  .LBB0_2: # %if.else
@@ -25,8 +24,7 @@ define void @br_fcmp_false(double %a, double %b) nounwind {
 ;
 ; RV64IFD-LABEL: br_fcmp_false:
 ; RV64IFD:       # %bb.0:
-; RV64IFD-NEXT:    li a0, 1
-; RV64IFD-NEXT:    bnez a0, .LBB0_2
+; RV64IFD-NEXT:    beqz zero, .LBB0_2
 ; RV64IFD-NEXT:  # %bb.1: # %if.then
 ; RV64IFD-NEXT:    ret
 ; RV64IFD-NEXT:  .LBB0_2: # %if.else
@@ -36,8 +34,7 @@ define void @br_fcmp_false(double %a, double %b) nounwind {
 ;
 ; RV32IZFINXZDINX-LABEL: br_fcmp_false:
 ; RV32IZFINXZDINX:       # %bb.0:
-; RV32IZFINXZDINX-NEXT:    li a0, 1
-; RV32IZFINXZDINX-NEXT:    bnez a0, .LBB0_2
+; RV32IZFINXZDINX-NEXT:    beqz zero, .LBB0_2
 ; RV32IZFINXZDINX-NEXT:  # %bb.1: # %if.then
 ; RV32IZFINXZDINX-NEXT:    ret
 ; RV32IZFINXZDINX-NEXT:  .LBB0_2: # %if.else
@@ -47,8 +44,7 @@ define void @br_fcmp_false(double %a, double %b) nounwind {
 ;
 ; RV64IZFINXZDINX-LABEL: br_fcmp_false:
 ; RV64IZFINXZDINX:       # %bb.0:
-; RV64IZFINXZDINX-NEXT:    li a0, 1
-; RV64IZFINXZDINX-NEXT:    bnez a0, .LBB0_2
+; RV64IZFINXZDINX-NEXT:    beqz zero, .LBB0_2
 ; RV64IZFINXZDINX-NEXT:  # %bb.1: # %if.then
 ; RV64IZFINXZDINX-NEXT:    ret
 ; RV64IZFINXZDINX-NEXT:  .LBB0_2: # %if.else
@@ -897,8 +893,7 @@ if.then:
 define void @br_fcmp_true(double %a, double %b) nounwind {
 ; RV32IFD-LABEL: br_fcmp_true:
 ; RV32IFD:       # %bb.0:
-; RV32IFD-NEXT:    li a0, 1
-; RV32IFD-NEXT:    bnez a0, .LBB16_2
+; RV32IFD-NEXT:    beqz zero, .LBB16_2
 ; RV32IFD-NEXT:  # %bb.1: # %if.else
 ; RV32IFD-NEXT:    ret
 ; RV32IFD-NEXT:  .LBB16_2: # %if.then
@@ -908,8 +903,7 @@ define void @br_fcmp_true(double %a, double %b) nounwind {
 ;
 ; RV64IFD-LABEL: br_fcmp_true:
 ; RV64IFD:       # %bb.0:
-; RV64IFD-NEXT:    li a0, 1
-; RV64IFD-NEXT:    bnez a0, .LBB16_2
+; RV64IFD-NEXT:    beqz zero, .LBB16_2
 ; RV64IFD-NEXT:  # %bb.1: # %if.else
 ; RV64IFD-NEXT:    ret
 ; RV64IFD-NEXT:  .LBB16_2: # %if.then
@@ -919,8 +913,7 @@ define void @br_fcmp_true(double %a, double %b) nounwind {
 ;
 ; RV32IZFINXZDINX-LABEL: br_fcmp_true:
 ; RV32IZFINXZDINX:       # %bb.0:
-; RV32IZFINXZDINX-NEXT:    li a0, 1
-; RV32IZFINXZDINX-NEXT:    bnez a0, .LBB16_2
+; RV32IZFINXZDINX-NEXT:    beqz zero, .LBB16_2
 ; RV32IZFINXZDINX-NEXT:  # %bb.1: # %if.else
 ; RV32IZFINXZDINX-NEXT:    ret
 ; RV32IZFINXZDINX-NEXT:  .LBB16_2: # %if.then
@@ -930,8 +923,7 @@ define void @br_fcmp_true(double %a, double %b) nounwind {
 ;
 ; RV64IZFINXZDINX-LABEL: br_fcmp_true:
 ; RV64IZFINXZDINX:       # %bb.0:
-; RV64IZFINXZDINX-NEXT:    li a0, 1
-; RV64IZFINXZDINX-NEXT:    bnez a0, .LBB16_2
+; RV64IZFINXZDINX-NEXT:    beqz zero, .LBB16_2
 ; RV64IZFINXZDINX-NEXT:  # %bb.1: # %if.else
 ; RV64IZFINXZDINX-NEXT:    ret
 ; RV64IZFINXZDINX-NEXT:  .LBB16_2: # %if.then
diff --git a/llvm/test/CodeGen/RISCV/float-br-fcmp.ll b/llvm/test/CodeGen/RISCV/float-br-fcmp.ll
index 35caa627b57bc..00da1cc1c8bbe 100644
--- a/llvm/test/CodeGen/RISCV/float-br-fcmp.ll
+++ b/llvm/test/CodeGen/RISCV/float-br-fcmp.ll
@@ -15,8 +15,7 @@ declare float @dummy(float)
 define void @br_fcmp_false(float %a, float %b) nounwind {
 ; RV32IF-LABEL: br_fcmp_false:
 ; RV32IF:       # %bb.0:
-; RV32IF-NEXT:    li a0, 1
-; RV32IF-NEXT:    bnez a0, .LBB0_2
+; RV32IF-NEXT:    beqz zero, .LBB0_2
 ; RV32IF-NEXT:  # %bb.1: # %if.then
 ; RV32IF-NEXT:    ret
 ; RV32IF-NEXT:  .LBB0_2: # %if.else
@@ -26,8 +25,7 @@ define void @br_fcmp_false(float %a, float %b) nounwind {
 ;
 ; RV64IF-LABEL: br_fcmp_false:
 ; RV64IF:       # %bb.0:
-; RV64IF-NEXT:    li a0, 1
-; RV64IF-NEXT:    bnez a0, .LBB0_2
+; RV64IF-NEXT:    beqz zero, .LBB0_2
 ; RV64IF-NEXT:  # %bb.1: # %if.then
 ; RV64IF-NEXT:    ret
 ; RV64IF-NEXT:  .LBB0_2: # %if.else
@@ -37,8 +35,7 @@ define void @br_fcmp_false(float %a, float %b) nounwind {
 ;
 ; RV32IZFINX-LABEL: br_fcmp_false:
 ; RV32IZFINX:       # %bb.0:
-; RV32IZFINX-NEXT:    li a0, 1
-; RV32IZFINX-NEXT:    bnez a0, .LBB0_2
+; RV32IZFINX-NEXT:    beqz zero, .LBB0_2
 ; RV32IZFINX-NEXT:  # %bb.1: # %if.then
 ; RV32IZFINX-NEXT:    ret
 ; RV32IZFINX-NEXT:  .LBB0_2: # %if.else
@@ -48,8 +45,7 @@ define void @br_fcmp_false(float %a, float %b) nounwind {
 ;
 ; RV64IZFINX-LABEL: br_fcmp_false:
 ; RV64IZFINX:       # %bb.0:
-; RV64IZFINX-NEXT:    li a0, 1
-; RV64IZFINX-NEXT:    bnez a0, .LBB0_2
+; RV64IZFINX-NEXT:    beqz zero, .LBB0_2
 ; RV64IZFINX-NEXT:  # %bb.1: # %if.then
 ; RV64IZFINX-NEXT:    ret
 ; RV64IZFINX-NEXT:  .LBB0_2: # %if.else
@@ -898,8 +894,7 @@ if.then:
 define void @br_fcmp_true(float %a, float %b) nounwind {
 ; RV32IF-LABEL: br_fcmp_true:
 ; RV32IF:       # %bb.0:
-; RV32IF-NEXT:    li a0, 1
-; RV32IF-NEXT:    bnez a0, .LBB16_2
+; RV32IF-NEXT:    beqz zero, .LBB16_2
 ; RV32IF-NEXT:  # %bb.1: # %if.else
 ; RV32IF-NEXT:    ret
 ; RV32IF-NEXT:  .LBB16_2: # %if.then
@@ -909,8 +904,7 @@ define void @br_fcmp_true(float %a, float %b) nounwind {
 ;
 ; RV64IF-LABEL: br_fcmp_true:
 ; RV64IF:       # %bb.0:
-; RV64IF-NEXT:    li a0, 1
-; RV64IF-NEXT:    bnez a0, .LBB16_2
+; RV64IF-NEXT:    beqz zero, .LBB16_2
 ; RV64IF-NEXT:  # %bb.1: # %if.else
 ; RV64IF-NEXT:    ret
 ; RV64IF-NEXT:  .LBB16_2: # %if.then
@@ -920,8 +914,7 @@ define void @br_fcmp_true(float %a, float %b) nounwind {
 ;
 ; RV32IZFINX-LABEL: br_fcmp_true:
 ; RV32IZFINX:       # %bb.0:
-; RV32IZFINX-NEXT:    li a0, 1
-; RV32IZFINX-NEXT:    bnez a0, .LBB16_2
+; RV32IZFINX-NEXT:    beqz zero, .LBB16_2
 ; RV32IZFINX-NEXT:  # %bb.1: # %if.else
 ; RV32IZFINX-NEXT:    ret
 ; RV32IZFINX-NEXT:  .LBB16_2: # %if.then
@@ -931,8 +924,7 @@ define void @br_fcmp_true(float %a, float %b) nounwind {
 ;
 ; RV64IZFINX-LABEL: br_fcmp_true:
 ; RV64IZFINX:       # %bb.0:
-; RV64IZFINX-NEXT:    li a0, 1
-; RV64IZFINX-NEXT:    bnez a0, .LBB16_2
+; RV64IZFINX-NEXT:    beqz zero, .LBB16_2
 ; RV64IZFINX-NEXT:  # %bb.1: # %if.else
 ; RV64IZFINX-NEXT:    ret
 ; RV64IZFINX-NEXT:  .LBB16_2: # %if.then
diff --git a/llvm/test/CodeGen/RISCV/half-br-fcmp.ll b/llvm/test/CodeGen/RISCV/half-br-fcmp.ll
index e9b142e33362f..1cee927662c14 100644
--- a/llvm/test/CodeGen/RISCV/half-br-fcmp.ll
+++ b/llvm/test/CodeGen/RISCV/half-br-fcmp.ll
@@ -23,8 +23,7 @@ declare half @dummy(half)
 define void @br_fcmp_false(half %a, half %b) nounwind {
 ; RV32IZFH-LABEL: br_fcmp_false:
 ; RV32IZFH:       # %bb.0:
-; RV32IZFH-NEXT:    li a0, 1
-; RV32IZFH-NEXT:    bnez a0, .LBB0_2
+; RV32IZFH-NEXT:    beqz zero, .LBB0_2
 ; RV32IZFH-NEXT:  # %bb.1: # %if.then
 ; RV32IZFH-NEXT:    ret
 ; RV32IZFH-NEXT:  .LBB0_2: # %if.else
@@ -34,8 +33,7 @@ define void @br_fcmp_false(half %a, half %b) nounwind {
 ;
 ; RV64IZFH-LABEL: br_fcmp_false:
 ; RV64IZFH:       # %bb.0:
-; RV64IZFH-NEXT:    li a0, 1
-; RV64IZFH-NEXT:    bnez a0, .LBB0_2
+; RV64IZFH-NEXT:    beqz zero, .LBB0_2
 ; RV64IZFH-NEXT:  # %bb.1: # %if.then
 ; RV64IZFH-NEXT:    ret
 ; RV64IZFH-NEXT:  .LBB0_2: # %if.else
@@ -45,8 +43,7 @@ define void @br_fcmp_false(half %a, half %b) nounwind {
 ;
 ; RV32IZHINX-LABEL: br_fcmp_false:
 ; RV32IZHINX:       # %bb.0:
-; RV32IZHINX-NEXT:    li a0, 1
-; RV32IZHINX-NEXT:    bnez a0, .LBB0_2
+; RV32IZHINX-NEXT:    beqz zero, .LBB0_2
 ; RV32IZHINX-NEXT:  # %bb.1: # %if.then
 ; RV32IZHINX-NEXT:    ret
 ; RV32IZHINX-NEXT:  .LBB0_2: # %if.else
@@ -56,8 +53,7 @@ define void @br_fcmp_false(half %a, half %b) nounwind {
 ;
 ; RV64IZHINX-LABEL: br_fcmp_false:
 ; RV64IZHINX:       # %bb.0:
-; RV64IZHINX-NEXT:    li a0, 1
-; RV64IZHINX-NEXT:    bnez a0, .LBB0_2
+; RV64IZHINX-NEXT:    beqz zero, .LBB0_2
 ; RV64IZHINX-NEXT:  # %bb.1: # %if.then
 ; RV64IZHINX-NEXT:    ret
 ; RV64IZHINX-NEXT:  .LBB0_2: # %if.else
@@ -67,8 +63,7 @@ define void @br_fcmp_false(half %a, half %b) nounwind {
 ;
 ; RV32IZFHMIN-LABEL: br_fcmp_false:
 ; RV32IZFHMIN:       # %bb.0:
-; RV32IZFHMIN-NEXT:    li a0, 1
-; RV32IZFHMIN-NEXT:    bnez a0, .LBB0_2
+; RV32IZFHMIN-NEXT:    beqz zero, .LBB0_2
 ; RV32IZFHMIN-NEXT:  # %bb.1: # %if.then
 ; RV32IZFHMIN-NEXT:    ret
 ; RV32IZFHMIN-NEXT:  .LBB0_2: # %if.else
@@ -78,8 +73,7 @@ define void @br_fcmp_false(half %a, half %b) nounwind {
 ;
 ; RV64IZFHMIN-LABEL: br_fcmp_false:
 ; RV64IZFHMIN:       # %bb.0:
-; RV64IZFHMIN-NEXT:    li a0, 1
-; RV64IZFHMIN-NEXT:    bnez a0, .LBB0_2
+; RV64IZFHMIN-NEXT:    beqz zero, .LBB0_2
 ; RV64IZFHMIN-NEXT:  # %bb.1: # %if.then
 ; RV64IZFHMIN-NEXT:    ret
 ; RV64IZFHMIN-NEXT:  .LBB0_2: # %if.else
@@ -89,8 +83,7 @@ define void @br_fcmp_false(half %a, half %b) nounwind {
 ;
 ; RV32IZHINXMIN-LABEL: br_fcmp_false:
 ; RV32IZHINXMIN:       # %bb.0:
-; RV32IZHINXMIN-NEXT:    li a0, 1
-; RV32IZHINXMIN-NEXT:    bnez a0, .LBB0_2
+; RV32IZHINXMIN-NEXT:    beqz zero, .LBB0_2
 ; RV32IZHINXMIN-NEXT:  # %bb.1: # %if.then
 ; RV32IZHINXMIN-NEXT:    ret
 ; RV32IZHINXMIN-NEXT:  .LBB0_2: # %if.else
@@ -100,8 +93,7 @@ define void @br_fcmp_false(half %a, half %b) nounwind {
 ;
 ; RV64IZHINXMIN-LABEL: br_fcmp_false:
 ; RV64IZHINXMIN:       # %bb.0:
-; RV64IZHINXMIN-NEXT:    li a0, 1
-; RV64IZHINXMIN-NEXT:    bnez a0, .LBB0_2
+; RV64IZHINXMIN-NEXT:    beqz zero, .LBB0_2
 ; RV64IZHINXMIN-NEXT:  # %bb.1: # %if.then
 ; RV64IZHINXMIN-NEXT:    ret
 ; RV64IZHINXMIN-NEXT:  .LBB0_2: # %if.else
@@ -1762,8 +1754,7 @@ if.then:
 define void @br_fcmp_true(half %a, half %b) nounwind {
 ; RV32IZFH-LABEL: br_fcmp_true:
 ; RV32IZFH:       # %bb.0:
-; RV32IZFH-NEXT:    li a0, 1
-; RV32IZFH-NEXT:    bnez a0, .LBB16_2
+; RV32IZFH-NEXT:    beqz zero, .LBB16_2
 ; RV32IZFH-NEXT:  # %bb.1: # %if.else
 ; RV32IZFH-NEXT:    ret
 ; RV32IZFH-NEXT:  .LBB16_2: # %if.then
@@ -1773,8 +1764,7 @@ define void @br_fcmp_true(half %a, half %b) nounwind {
 ;
 ; RV64IZFH-LABEL: br_fcmp_true:
 ; RV64IZFH:       # %bb.0:
-; RV64IZFH-NEXT:    li a0, 1
-; RV64IZFH-NEXT:    bnez a0, .LBB16_2
+; RV64IZFH-NEXT:    beqz zero, .LBB16_2
 ; RV64IZFH-NEXT:  # %bb.1: # %if.else
 ; RV64IZFH-NEXT:    ret
 ; RV64IZFH-NEXT:  .LBB16_2: # %if.then
@@ -1784,8 +1774,7 @@ define void @br_fcmp_true(half %a, half %b) nounwind {
 ;
 ; RV32IZHINX-LABEL: br_fcmp_true:
 ; RV32IZHINX:       # %bb.0:
-; RV32IZHINX-NEXT:    li a0, 1
-; RV32IZHINX-NEXT:    bnez a0, .LBB16_2
+; RV32IZHINX-NEXT:    beqz zero, .LBB16_2
 ; RV32IZHINX-NEXT:  # %bb.1: # %if.else
 ; RV32IZHINX-NEXT:    ret
 ; RV32IZHINX-NEXT:  .LBB16_2: # %if.then
@@ -1795,8 +1784,7 @@ define void @br_fcmp_true(half %a, half %b) nounwind {
 ;
 ; RV64IZHINX-LABEL: br_fcmp_true:
 ; RV64IZHINX:       # %bb.0:
-; RV64IZHINX-NEXT:    li a0, 1
-; RV64IZHINX-NEXT:    bnez a0, .LBB16_2
+; RV64IZHINX-NEXT:    beqz zero, .LBB16_2
 ; RV64IZHINX-NEXT:  # %bb.1: # %if.else
 ; RV64IZHINX-NEXT:    ret
 ; RV64IZHINX-NEXT:  .LBB16_2: # %if.then
@@ -1806,8 +1794,7 @@ define void @br_fcmp_true(half %a, half %b) nounwind {
 ;
 ; RV32IZFHMIN-LABEL: br_fcmp_true:
 ; RV32IZFHMIN:       # %bb.0:
-; RV32IZFHMIN-NEXT:    li a0, 1
-; RV32IZFHMIN-NEXT:    bnez a0, .LBB16_2
+; RV32IZFHMIN-NEXT:    beqz zero, .LBB16_2
 ; RV32IZFHMIN-NEXT:  # %bb.1: # %if.else
 ; RV32IZFHMIN-NEXT:    ret
 ; RV32IZFHMIN-NEXT:  .LBB16_2: # %if.then
@@ -1817,8 +1804,7 @@ define void @br_fcmp_true(half %a, half %b) nounwind {
 ;
 ; RV64IZFHMIN-LABEL: br_fcmp_true:
 ; RV64IZFHMIN:       # %bb.0:
-; RV64IZFHMIN-NEXT:    li a0, 1
-; RV64IZFHMIN-NEXT:    bnez a0, .LBB16_2
+; RV64IZFHMIN-NEXT:    beqz zero, .LBB16_2
 ; RV64IZFHMIN-NEXT:  # %bb.1: # %if.else
 ; RV64IZFHMIN-NEXT:    ret
 ; RV64IZFHMIN-NEXT:  .LBB16_2: # %if.then
@@ -1828,8 +1814,7 @@ define void @br_fcmp_true(half %a, half %b) nounwind {
 ;
 ; RV32IZHINXMIN-LABEL: br_fcmp_true:
 ; RV32IZHINXMIN:       # %bb.0:
-; RV32IZHINXMIN-NEXT:    li a0, 1
-; RV32IZHINXMIN-NEXT:    bnez a0, .LBB16_2
+; RV32IZHINXMIN-NEXT:    beqz zero, .LBB16_2
 ; RV32IZHINXMIN-NEXT:  # %bb.1: # %if.else
 ; RV32IZHINXMIN-NEXT:    ret
 ; RV32IZHINXMIN-NEXT:  .LBB16_2: # %if.then
@@ -1839,8 +1824,7 @@ define void @br_fcmp_true(half %a, half %b) nounwind {
 ;
 ; RV64IZHINXMIN-LABEL: br_fcmp_true:
 ; RV64IZHINXMIN:       # %bb.0:
-; RV64IZHINXMIN-NEXT:    li a0, 1
-; RV64IZHINXMIN-NEXT:    bnez a0, .LBB16_2
+; RV64IZHINXMIN-NEXT:    beqz zero, .LBB16_2
 ; RV64IZHINXMIN-NEXT:  # %bb.1: # %if.else
 ; RV64IZHINXMIN-NEXT:    ret
 ; RV64IZHINXMIN-NEXT:  .LBB16_2: # %if.then
diff --git a/llvm/test/CodeGen/RISCV/machine-sink-load-immediate.ll b/llvm/test/CodeGen/RISCV/machine-sink-load-immediate.ll
index eb84774014a4b..4947423971c6b 100644
--- a/llvm/test/CodeGen/RISCV/machine-sink-load-immediate.ll
+++ b/llvm/test/CodeGen/RISCV/machine-sink-load-immediate.ll
@@ -13,76 +13,72 @@ define i1 @sink_li(ptr %text, ptr %text.addr.0) nounwind {
 ; CHECK-NEXT:    mv s0, a0
 ; CHECK-NEXT:    call toupper
 ; CHECK-NEXT:    li a1, 0
-; CHECK-NEXT:    beqz s0, .LBB0_26
-; CHECK-NEXT:  # %bb.1: # %while.body.preheader
-; CHECK-NEXT:    li a2, 1
-; CHECK-NEXT:    li a3, 9
-; CHECK-NEXT:    li a4, 32
-; CHECK-NEXT:  .LBB0_2: # %while.body
+; CHECK-NEXT:    beqz s0, .LBB0_25
+; CHECK-NEXT:  .LBB0_1: # %while.body
 ; CHECK-NEXT:    # =>This Inner Loop Header: Depth=1
-; CHECK-NEXT:    bnez a2, .LBB0_4
-; CHECK-NEXT:  # %bb.3: # %while.body
-; CHECK-...
[truncated]

Copy link

github-actions bot commented Mar 25, 2025

✅ With the latest revision this PR passed the C/C++ code formatter.

Copy link
Contributor

@michaelmaitland michaelmaitland left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This patch will be helpful for dynamic instruction count since we avoid the load. For that reason, I am inclined to accept this patch. However, there are two related issues that are still left:

  1. We may miss the opportunity to canonicalize if the li + condbr pair is introduced later on in the pipeline. I am not aware of whether this occurs or not.
  2. We miss the opportunity to turn these branches into an unconditional jump. I wonder if this kind of optimization would be beneficial for branch prediction. Maybe the a LatePeephole pass could just look to modify the CFG if it sees this canonical case.

If (1) does not occur, then I think it is appropriate to proceed forward with accepting this patch, followed up with the suggestion in (2), and there is no need to evaluate the conditional branch statically during a late peephole pass. However, if we are introducing these pairs later in the pipeline, I still think this patch should be landed since we may benefit from optimizing earlier and then the late peephole pass will need to both evaluate the condition and simplify the branch (and update CFG).

WDYT?

Copy link
Member

@mikhailramalho mikhailramalho left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with Michael. This PR + a late peephole pass will definitely be beneficial.

@preames preames merged commit 8742022 into llvm:main Mar 27, 2025
11 checks passed
@preames preames deleted the pr-riscv-canonicalize-branch-cond branch March 27, 2025 15:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants