Skip to content

Conversation

svs-quic
Copy link
Contributor

@svs-quic svs-quic commented Mar 5, 2025

The function currently only checks to see if we compare against an immediate before selecting the two branch immediate instructions that are a part of the XCVbi vendor extension. This works at the moment since there are no other extensions that have a branch immediate instruction, but it would be better if we explicitly check to see if the XCVbi extension is enabled before we return the appropriate instruction.

This is also done in preparation for the branch immediate instructions that are a part of the Xqcibi vendor extension from Qualcomm.

The function currently only checks to see if the we compare against an
immediate before selecting the two branch immediate instructions that are
a part of the XCVbi vendor extension. This works at the moment since there are
no other extensions that have a branch immediate instruction but it would be better
if we explicitly check to see if the XCVbi extension is enabled before we return
the appropriate instruction. This is also done in preparation for the branch immediate
instructions that are a part of the Xqcibi vendor extension from Qualcomm.
@llvmbot
Copy link
Member

llvmbot commented Mar 5, 2025

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

Author: Sudharsan Veeravalli (svs-quic)

Changes

The function currently only checks to see if the we compare against an immediate before selecting the two branch immediate instructions that are a part of the XCVbi vendor extension. This works at the moment since there are no other extensions that have a branch immediate instruction but it would be better if we explicitly check to see if the XCVbi extension is enabled before we return the appropriate instruction.

This is also done in preparation for the branch immediate instructions that are a part of the Xqcibi vendor extension from Qualcomm.


Full diff: https://github.com/llvm/llvm-project/pull/129859.diff

3 Files Affected:

  • (modified) llvm/lib/Target/RISCV/GISel/RISCVInstructionSelector.cpp (+2-1)
  • (modified) llvm/lib/Target/RISCV/RISCVInstrInfo.cpp (+13-4)
  • (modified) llvm/lib/Target/RISCV/RISCVInstrInfo.h (+2-1)
diff --git a/llvm/lib/Target/RISCV/GISel/RISCVInstructionSelector.cpp b/llvm/lib/Target/RISCV/GISel/RISCVInstructionSelector.cpp
index 62fbe55dffba1..3550ab4c00476 100644
--- a/llvm/lib/Target/RISCV/GISel/RISCVInstructionSelector.cpp
+++ b/llvm/lib/Target/RISCV/GISel/RISCVInstructionSelector.cpp
@@ -789,7 +789,8 @@ bool RISCVInstructionSelector::select(MachineInstr &MI) {
     RISCVCC::CondCode CC;
     getOperandsForBranch(MI.getOperand(0).getReg(), CC, LHS, RHS, *MRI);
 
-    auto Bcc = MIB.buildInstr(RISCVCC::getBrCond(CC), {}, {LHS, RHS})
+    auto Bcc = MIB.buildInstr(RISCVCC::getBrCond(CC, STI.getFeatureBits()), {},
+                              {LHS, RHS})
                    .addMBB(MI.getOperand(1).getMBB());
     MI.eraseFromParent();
     return constrainSelectedInstRegOperands(*Bcc, TII, TRI, RBI);
diff --git a/llvm/lib/Target/RISCV/RISCVInstrInfo.cpp b/llvm/lib/Target/RISCV/RISCVInstrInfo.cpp
index 913169c00b642..748b072e64969 100644
--- a/llvm/lib/Target/RISCV/RISCVInstrInfo.cpp
+++ b/llvm/lib/Target/RISCV/RISCVInstrInfo.cpp
@@ -953,14 +953,23 @@ static void parseCondBranch(MachineInstr &LastInst, MachineBasicBlock *&Target,
   Cond.push_back(LastInst.getOperand(1));
 }
 
-unsigned RISCVCC::getBrCond(RISCVCC::CondCode CC, bool Imm) {
+unsigned RISCVCC::getBrCond(RISCVCC::CondCode CC,
+                            const FeatureBitset &FeatureBits, bool Imm) {
   switch (CC) {
   default:
     llvm_unreachable("Unknown condition code!");
   case RISCVCC::COND_EQ:
-    return Imm ? RISCV::CV_BEQIMM : RISCV::BEQ;
+    if (Imm && FeatureBits[RISCV::FeatureVendorXCVbi])
+      return RISCV::CV_BEQIMM;
+    else
+      llvm_unreachable("Unknown branch immediate!");
+    return RISCV::BEQ;
   case RISCVCC::COND_NE:
-    return Imm ? RISCV::CV_BNEIMM : RISCV::BNE;
+    if (Imm && FeatureBits[RISCV::FeatureVendorXCVbi])
+      return RISCV::CV_BNEIMM;
+    else
+      llvm_unreachable("Unknown branch immediate!");
+    return RISCV::BNE;
   case RISCVCC::COND_LT:
     return RISCV::BLT;
   case RISCVCC::COND_GE:
@@ -974,7 +983,7 @@ unsigned RISCVCC::getBrCond(RISCVCC::CondCode CC, bool Imm) {
 
 const MCInstrDesc &RISCVInstrInfo::getBrCond(RISCVCC::CondCode CC,
                                              bool Imm) const {
-  return get(RISCVCC::getBrCond(CC, Imm));
+  return get(RISCVCC::getBrCond(CC, STI.getFeatureBits(), Imm));
 }
 
 RISCVCC::CondCode RISCVCC::getOppositeBranchCondition(RISCVCC::CondCode CC) {
diff --git a/llvm/lib/Target/RISCV/RISCVInstrInfo.h b/llvm/lib/Target/RISCV/RISCVInstrInfo.h
index 1c46d761a7e1e..0074c837eebfe 100644
--- a/llvm/lib/Target/RISCV/RISCVInstrInfo.h
+++ b/llvm/lib/Target/RISCV/RISCVInstrInfo.h
@@ -45,7 +45,8 @@ enum CondCode {
 };
 
 CondCode getOppositeBranchCondition(CondCode);
-unsigned getBrCond(CondCode CC, bool Imm = false);
+unsigned getBrCond(CondCode CC, const FeatureBitset &FeatureBits,
+                   bool Imm = false);
 
 } // end of namespace RISCVCC
 

@svs-quic svs-quic changed the title [RISCV] Check feature bits in getBrCond [RISCV] Check subtarget feature in getBrCond Mar 5, 2025
@svs-quic svs-quic requested a review from lenary March 5, 2025 12:38
@svs-quic svs-quic merged commit 6262d67 into llvm:main Mar 5, 2025
11 checks passed
@svs-quic svs-quic deleted the beqimm branch March 6, 2025 07:27
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.

3 participants