Skip to content

[RISCV] Check subtarget feature in getBrCond #129859

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 3 commits into from
Mar 5, 2025
Merged

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
jph-13 pushed a commit to jph-13/llvm-project that referenced this pull request Mar 21, 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.
It would be better if we explicitly check if the XCVbi extension is enabled
before returning 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.
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