Skip to content

[RISCV] Add Tied operands in Xqcicm instructions and changes to handle a single tied operand in source DAG and instruction #145538

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
Jun 25, 2025

Conversation

hchandel
Copy link
Contributor

Tied Operands change is required for adding codegen patterns for Qualcomm uC Xqcicm instructions
which will be done in a follow-up PR. This change leads to one of instructions getting compressed even
when it shouldn't be. This case was not covered in #143660. Added changes to correctly handle this case.

Harsh Chandel added 2 commits June 24, 2025 12:54
This is required for adding codegen patterns for Xqcicm instructions
which will be done in a follow-up PR. This change leads to one of
instructions getting compressed even when it shouldn't be.

Change-Id: I945255ecacec8b8ac0f4c5356fdefdd14269d396
… instruction

This case was not covered in llvm#143660. Added changes to correctly handle this case.

Change-Id: I40006a35e505c1e09b8ca4eadf4a85451d4e769b
@hchandel hchandel requested a review from topperc June 24, 2025 15:59
@llvmbot llvmbot added backend:RISC-V tablegen mc Machine (object) code labels Jun 24, 2025
@llvmbot
Copy link
Member

llvmbot commented Jun 24, 2025

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

Author: quic_hchandel (hchandel)

Changes

Tied Operands change is required for adding codegen patterns for Qualcomm uC Xqcicm instructions
which will be done in a follow-up PR. This change leads to one of instructions getting compressed even
when it shouldn't be. This case was not covered in #143660. Added changes to correctly handle this case.


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

3 Files Affected:

  • (modified) llvm/lib/Target/RISCV/RISCVInstrInfoXqci.td (+9-6)
  • (modified) llvm/test/MC/RISCV/xqcicm-valid.s (+6)
  • (modified) llvm/utils/TableGen/CompressInstEmitter.cpp (+11-4)
diff --git a/llvm/lib/Target/RISCV/RISCVInstrInfoXqci.td b/llvm/lib/Target/RISCV/RISCVInstrInfoXqci.td
index b52798edbe143..ccd20859c24e6 100644
--- a/llvm/lib/Target/RISCV/RISCVInstrInfoXqci.td
+++ b/llvm/lib/Target/RISCV/RISCVInstrInfoXqci.td
@@ -596,17 +596,20 @@ class QCILICC<bits<3> funct3, bits<2> funct2, DAGOperand InTyRs2, string opcodes
 
 let hasSideEffects = 0, mayLoad = 0, mayStore = 0 in
 class QCIMVCC<bits<3> funct3, string opcodestr>
-    : RVInstR4<0b00, funct3, OPC_CUSTOM_2, (outs GPRNoX0:$rd),
-               (ins GPRNoX0:$rs1, GPRNoX0:$rs2, GPRNoX0:$rs3),
-               opcodestr, "$rd, $rs1, $rs2, $rs3">;
+    : RVInstR4<0b00, funct3, OPC_CUSTOM_2, (outs GPRNoX0:$rd_wb),
+               (ins GPRNoX0:$rd, GPRNoX0:$rs1, GPRNoX0:$rs2, GPRNoX0:$rs3),
+               opcodestr, "$rd, $rs1, $rs2, $rs3"> {
+  let Constraints = "$rd = $rd_wb";
+}
 
 let hasSideEffects = 0, mayLoad = 0, mayStore = 0 in
 class QCIMVCCI<bits<3> funct3, string opcodestr, DAGOperand immType>
-    : RVInstR4<0b10, funct3, OPC_CUSTOM_2, (outs GPRNoX0:$rd),
-               (ins GPRNoX0:$rs1, immType:$imm, GPRNoX0:$rs3),
+    : RVInstR4<0b10, funct3, OPC_CUSTOM_2, (outs GPRNoX0:$rd_wb),
+               (ins GPRNoX0:$rd, GPRNoX0:$rs1, immType:$imm, GPRNoX0:$rs3),
                opcodestr, "$rd, $rs1, $imm, $rs3"> {
   bits<5> imm;
-
+  
+  let Constraints = "$rd = $rd_wb";
   let rs2 = imm;
 }
 
diff --git a/llvm/test/MC/RISCV/xqcicm-valid.s b/llvm/test/MC/RISCV/xqcicm-valid.s
index 0eb253ff5e434..a9ce30e150482 100644
--- a/llvm/test/MC/RISCV/xqcicm-valid.s
+++ b/llvm/test/MC/RISCV/xqcicm-valid.s
@@ -135,3 +135,9 @@ qc.mveqi x9, x9, 0, x12
 # CHECK-ENC: encoding: [0x06,0xae]
 qc.mvltui x9, x9, 1, x12
 
+# Following instruction should not be compressed
+
+# CHECK-INST: qc.mveqi a0, s1, 0, a2
+# CHECK-ENC: encoding: [0x5b,0x85,0x04,0x64]
+qc.mveqi x10, x9, 0, x12
+
diff --git a/llvm/utils/TableGen/CompressInstEmitter.cpp b/llvm/utils/TableGen/CompressInstEmitter.cpp
index 2dfeea36e2134..8c850ffc417f1 100644
--- a/llvm/utils/TableGen/CompressInstEmitter.cpp
+++ b/llvm/utils/TableGen/CompressInstEmitter.cpp
@@ -137,7 +137,8 @@ class CompressInstEmitter {
                                StringMap<unsigned> &SourceOperands,
                                StringMap<unsigned> &DestOperands,
                                const DagInit *SourceDag, const DagInit *DestDag,
-                               IndexedMap<OpData> &SourceOperandMap);
+                               IndexedMap<OpData> &SourceOperandMap,
+                               bool HasSourceTiedOp);
 
   void createInstOperandMapping(const Record *Rec, const DagInit *SourceDag,
                                 const DagInit *DestDag,
@@ -349,7 +350,8 @@ static bool validateArgsTypes(const Init *Arg1, const Init *Arg2) {
 void CompressInstEmitter::createDagOperandMapping(
     const Record *Rec, StringMap<unsigned> &SourceOperands,
     StringMap<unsigned> &DestOperands, const DagInit *SourceDag,
-    const DagInit *DestDag, IndexedMap<OpData> &SourceOperandMap) {
+    const DagInit *DestDag, IndexedMap<OpData> &SourceOperandMap,
+    bool HasSourceTiedOp) {
   for (unsigned I = 0; I < DestDag->getNumArgs(); ++I) {
     // Skip fixed immediates and registers, they were handled in
     // addDagOperandMapping.
@@ -368,7 +370,10 @@ void CompressInstEmitter::createDagOperandMapping(
         SourceOperands.find(SourceDag->getArgNameStr(I));
     if (It != SourceOperands.end()) {
       // Operand sharing the same name in the Dag should be mapped as tied.
-      SourceOperandMap[I].TiedOpIdx = It->getValue();
+      if (HasSourceTiedOp)
+        SourceOperandMap[I + 1].TiedOpIdx = It->getValue() + 1;
+      else
+        SourceOperandMap[I].TiedOpIdx = It->getValue();
       if (!validateArgsTypes(SourceDag->getArg(It->getValue()),
                              SourceDag->getArg(I)))
         PrintFatalError(Rec->getLoc(),
@@ -521,8 +526,10 @@ void CompressInstEmitter::evaluateCompressPat(const Record *Rec) {
 
   StringMap<unsigned> SourceOperands;
   StringMap<unsigned> DestOperands;
+  bool HasSourceTiedOp =
+      SourceLastTiedOp != std::numeric_limits<unsigned int>::max();
   createDagOperandMapping(Rec, SourceOperands, DestOperands, SourceDag, DestDag,
-                          SourceOperandMap);
+                          SourceOperandMap, HasSourceTiedOp);
   // Create operand mapping between the source and destination instructions.
   createInstOperandMapping(Rec, SourceDag, DestDag, SourceOperandMap,
                            DestOperandMap, SourceOperands, DestInst,

@llvmbot
Copy link
Member

llvmbot commented Jun 24, 2025

@llvm/pr-subscribers-tablegen

Author: quic_hchandel (hchandel)

Changes

Tied Operands change is required for adding codegen patterns for Qualcomm uC Xqcicm instructions
which will be done in a follow-up PR. This change leads to one of instructions getting compressed even
when it shouldn't be. This case was not covered in #143660. Added changes to correctly handle this case.


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

3 Files Affected:

  • (modified) llvm/lib/Target/RISCV/RISCVInstrInfoXqci.td (+9-6)
  • (modified) llvm/test/MC/RISCV/xqcicm-valid.s (+6)
  • (modified) llvm/utils/TableGen/CompressInstEmitter.cpp (+11-4)
diff --git a/llvm/lib/Target/RISCV/RISCVInstrInfoXqci.td b/llvm/lib/Target/RISCV/RISCVInstrInfoXqci.td
index b52798edbe143..ccd20859c24e6 100644
--- a/llvm/lib/Target/RISCV/RISCVInstrInfoXqci.td
+++ b/llvm/lib/Target/RISCV/RISCVInstrInfoXqci.td
@@ -596,17 +596,20 @@ class QCILICC<bits<3> funct3, bits<2> funct2, DAGOperand InTyRs2, string opcodes
 
 let hasSideEffects = 0, mayLoad = 0, mayStore = 0 in
 class QCIMVCC<bits<3> funct3, string opcodestr>
-    : RVInstR4<0b00, funct3, OPC_CUSTOM_2, (outs GPRNoX0:$rd),
-               (ins GPRNoX0:$rs1, GPRNoX0:$rs2, GPRNoX0:$rs3),
-               opcodestr, "$rd, $rs1, $rs2, $rs3">;
+    : RVInstR4<0b00, funct3, OPC_CUSTOM_2, (outs GPRNoX0:$rd_wb),
+               (ins GPRNoX0:$rd, GPRNoX0:$rs1, GPRNoX0:$rs2, GPRNoX0:$rs3),
+               opcodestr, "$rd, $rs1, $rs2, $rs3"> {
+  let Constraints = "$rd = $rd_wb";
+}
 
 let hasSideEffects = 0, mayLoad = 0, mayStore = 0 in
 class QCIMVCCI<bits<3> funct3, string opcodestr, DAGOperand immType>
-    : RVInstR4<0b10, funct3, OPC_CUSTOM_2, (outs GPRNoX0:$rd),
-               (ins GPRNoX0:$rs1, immType:$imm, GPRNoX0:$rs3),
+    : RVInstR4<0b10, funct3, OPC_CUSTOM_2, (outs GPRNoX0:$rd_wb),
+               (ins GPRNoX0:$rd, GPRNoX0:$rs1, immType:$imm, GPRNoX0:$rs3),
                opcodestr, "$rd, $rs1, $imm, $rs3"> {
   bits<5> imm;
-
+  
+  let Constraints = "$rd = $rd_wb";
   let rs2 = imm;
 }
 
diff --git a/llvm/test/MC/RISCV/xqcicm-valid.s b/llvm/test/MC/RISCV/xqcicm-valid.s
index 0eb253ff5e434..a9ce30e150482 100644
--- a/llvm/test/MC/RISCV/xqcicm-valid.s
+++ b/llvm/test/MC/RISCV/xqcicm-valid.s
@@ -135,3 +135,9 @@ qc.mveqi x9, x9, 0, x12
 # CHECK-ENC: encoding: [0x06,0xae]
 qc.mvltui x9, x9, 1, x12
 
+# Following instruction should not be compressed
+
+# CHECK-INST: qc.mveqi a0, s1, 0, a2
+# CHECK-ENC: encoding: [0x5b,0x85,0x04,0x64]
+qc.mveqi x10, x9, 0, x12
+
diff --git a/llvm/utils/TableGen/CompressInstEmitter.cpp b/llvm/utils/TableGen/CompressInstEmitter.cpp
index 2dfeea36e2134..8c850ffc417f1 100644
--- a/llvm/utils/TableGen/CompressInstEmitter.cpp
+++ b/llvm/utils/TableGen/CompressInstEmitter.cpp
@@ -137,7 +137,8 @@ class CompressInstEmitter {
                                StringMap<unsigned> &SourceOperands,
                                StringMap<unsigned> &DestOperands,
                                const DagInit *SourceDag, const DagInit *DestDag,
-                               IndexedMap<OpData> &SourceOperandMap);
+                               IndexedMap<OpData> &SourceOperandMap,
+                               bool HasSourceTiedOp);
 
   void createInstOperandMapping(const Record *Rec, const DagInit *SourceDag,
                                 const DagInit *DestDag,
@@ -349,7 +350,8 @@ static bool validateArgsTypes(const Init *Arg1, const Init *Arg2) {
 void CompressInstEmitter::createDagOperandMapping(
     const Record *Rec, StringMap<unsigned> &SourceOperands,
     StringMap<unsigned> &DestOperands, const DagInit *SourceDag,
-    const DagInit *DestDag, IndexedMap<OpData> &SourceOperandMap) {
+    const DagInit *DestDag, IndexedMap<OpData> &SourceOperandMap,
+    bool HasSourceTiedOp) {
   for (unsigned I = 0; I < DestDag->getNumArgs(); ++I) {
     // Skip fixed immediates and registers, they were handled in
     // addDagOperandMapping.
@@ -368,7 +370,10 @@ void CompressInstEmitter::createDagOperandMapping(
         SourceOperands.find(SourceDag->getArgNameStr(I));
     if (It != SourceOperands.end()) {
       // Operand sharing the same name in the Dag should be mapped as tied.
-      SourceOperandMap[I].TiedOpIdx = It->getValue();
+      if (HasSourceTiedOp)
+        SourceOperandMap[I + 1].TiedOpIdx = It->getValue() + 1;
+      else
+        SourceOperandMap[I].TiedOpIdx = It->getValue();
       if (!validateArgsTypes(SourceDag->getArg(It->getValue()),
                              SourceDag->getArg(I)))
         PrintFatalError(Rec->getLoc(),
@@ -521,8 +526,10 @@ void CompressInstEmitter::evaluateCompressPat(const Record *Rec) {
 
   StringMap<unsigned> SourceOperands;
   StringMap<unsigned> DestOperands;
+  bool HasSourceTiedOp =
+      SourceLastTiedOp != std::numeric_limits<unsigned int>::max();
   createDagOperandMapping(Rec, SourceOperands, DestOperands, SourceDag, DestDag,
-                          SourceOperandMap);
+                          SourceOperandMap, HasSourceTiedOp);
   // Create operand mapping between the source and destination instructions.
   createInstOperandMapping(Rec, SourceDag, DestDag, SourceOperandMap,
                            DestOperandMap, SourceOperands, DestInst,

@llvmbot
Copy link
Member

llvmbot commented Jun 24, 2025

@llvm/pr-subscribers-mc

Author: quic_hchandel (hchandel)

Changes

Tied Operands change is required for adding codegen patterns for Qualcomm uC Xqcicm instructions
which will be done in a follow-up PR. This change leads to one of instructions getting compressed even
when it shouldn't be. This case was not covered in #143660. Added changes to correctly handle this case.


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

3 Files Affected:

  • (modified) llvm/lib/Target/RISCV/RISCVInstrInfoXqci.td (+9-6)
  • (modified) llvm/test/MC/RISCV/xqcicm-valid.s (+6)
  • (modified) llvm/utils/TableGen/CompressInstEmitter.cpp (+11-4)
diff --git a/llvm/lib/Target/RISCV/RISCVInstrInfoXqci.td b/llvm/lib/Target/RISCV/RISCVInstrInfoXqci.td
index b52798edbe143..ccd20859c24e6 100644
--- a/llvm/lib/Target/RISCV/RISCVInstrInfoXqci.td
+++ b/llvm/lib/Target/RISCV/RISCVInstrInfoXqci.td
@@ -596,17 +596,20 @@ class QCILICC<bits<3> funct3, bits<2> funct2, DAGOperand InTyRs2, string opcodes
 
 let hasSideEffects = 0, mayLoad = 0, mayStore = 0 in
 class QCIMVCC<bits<3> funct3, string opcodestr>
-    : RVInstR4<0b00, funct3, OPC_CUSTOM_2, (outs GPRNoX0:$rd),
-               (ins GPRNoX0:$rs1, GPRNoX0:$rs2, GPRNoX0:$rs3),
-               opcodestr, "$rd, $rs1, $rs2, $rs3">;
+    : RVInstR4<0b00, funct3, OPC_CUSTOM_2, (outs GPRNoX0:$rd_wb),
+               (ins GPRNoX0:$rd, GPRNoX0:$rs1, GPRNoX0:$rs2, GPRNoX0:$rs3),
+               opcodestr, "$rd, $rs1, $rs2, $rs3"> {
+  let Constraints = "$rd = $rd_wb";
+}
 
 let hasSideEffects = 0, mayLoad = 0, mayStore = 0 in
 class QCIMVCCI<bits<3> funct3, string opcodestr, DAGOperand immType>
-    : RVInstR4<0b10, funct3, OPC_CUSTOM_2, (outs GPRNoX0:$rd),
-               (ins GPRNoX0:$rs1, immType:$imm, GPRNoX0:$rs3),
+    : RVInstR4<0b10, funct3, OPC_CUSTOM_2, (outs GPRNoX0:$rd_wb),
+               (ins GPRNoX0:$rd, GPRNoX0:$rs1, immType:$imm, GPRNoX0:$rs3),
                opcodestr, "$rd, $rs1, $imm, $rs3"> {
   bits<5> imm;
-
+  
+  let Constraints = "$rd = $rd_wb";
   let rs2 = imm;
 }
 
diff --git a/llvm/test/MC/RISCV/xqcicm-valid.s b/llvm/test/MC/RISCV/xqcicm-valid.s
index 0eb253ff5e434..a9ce30e150482 100644
--- a/llvm/test/MC/RISCV/xqcicm-valid.s
+++ b/llvm/test/MC/RISCV/xqcicm-valid.s
@@ -135,3 +135,9 @@ qc.mveqi x9, x9, 0, x12
 # CHECK-ENC: encoding: [0x06,0xae]
 qc.mvltui x9, x9, 1, x12
 
+# Following instruction should not be compressed
+
+# CHECK-INST: qc.mveqi a0, s1, 0, a2
+# CHECK-ENC: encoding: [0x5b,0x85,0x04,0x64]
+qc.mveqi x10, x9, 0, x12
+
diff --git a/llvm/utils/TableGen/CompressInstEmitter.cpp b/llvm/utils/TableGen/CompressInstEmitter.cpp
index 2dfeea36e2134..8c850ffc417f1 100644
--- a/llvm/utils/TableGen/CompressInstEmitter.cpp
+++ b/llvm/utils/TableGen/CompressInstEmitter.cpp
@@ -137,7 +137,8 @@ class CompressInstEmitter {
                                StringMap<unsigned> &SourceOperands,
                                StringMap<unsigned> &DestOperands,
                                const DagInit *SourceDag, const DagInit *DestDag,
-                               IndexedMap<OpData> &SourceOperandMap);
+                               IndexedMap<OpData> &SourceOperandMap,
+                               bool HasSourceTiedOp);
 
   void createInstOperandMapping(const Record *Rec, const DagInit *SourceDag,
                                 const DagInit *DestDag,
@@ -349,7 +350,8 @@ static bool validateArgsTypes(const Init *Arg1, const Init *Arg2) {
 void CompressInstEmitter::createDagOperandMapping(
     const Record *Rec, StringMap<unsigned> &SourceOperands,
     StringMap<unsigned> &DestOperands, const DagInit *SourceDag,
-    const DagInit *DestDag, IndexedMap<OpData> &SourceOperandMap) {
+    const DagInit *DestDag, IndexedMap<OpData> &SourceOperandMap,
+    bool HasSourceTiedOp) {
   for (unsigned I = 0; I < DestDag->getNumArgs(); ++I) {
     // Skip fixed immediates and registers, they were handled in
     // addDagOperandMapping.
@@ -368,7 +370,10 @@ void CompressInstEmitter::createDagOperandMapping(
         SourceOperands.find(SourceDag->getArgNameStr(I));
     if (It != SourceOperands.end()) {
       // Operand sharing the same name in the Dag should be mapped as tied.
-      SourceOperandMap[I].TiedOpIdx = It->getValue();
+      if (HasSourceTiedOp)
+        SourceOperandMap[I + 1].TiedOpIdx = It->getValue() + 1;
+      else
+        SourceOperandMap[I].TiedOpIdx = It->getValue();
       if (!validateArgsTypes(SourceDag->getArg(It->getValue()),
                              SourceDag->getArg(I)))
         PrintFatalError(Rec->getLoc(),
@@ -521,8 +526,10 @@ void CompressInstEmitter::evaluateCompressPat(const Record *Rec) {
 
   StringMap<unsigned> SourceOperands;
   StringMap<unsigned> DestOperands;
+  bool HasSourceTiedOp =
+      SourceLastTiedOp != std::numeric_limits<unsigned int>::max();
   createDagOperandMapping(Rec, SourceOperands, DestOperands, SourceDag, DestDag,
-                          SourceOperandMap);
+                          SourceOperandMap, HasSourceTiedOp);
   // Create operand mapping between the source and destination instructions.
   createInstOperandMapping(Rec, SourceDag, DestDag, SourceOperandMap,
                            DestOperandMap, SourceOperands, DestInst,

@hchandel hchandel requested review from lenary, pgodeq and svs-quic June 24, 2025 16:00
Copy link
Collaborator

@topperc topperc left a comment

Choose a reason for hiding this comment

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

LGTM

@hchandel hchandel merged commit deb3464 into llvm:main Jun 25, 2025
11 checks passed
@hchandel hchandel deleted the cm_tied branch June 25, 2025 04:55
anthonyhatran pushed a commit to anthonyhatran/llvm-project that referenced this pull request Jun 26, 2025
…e a single tied operand in source DAG and instruction (llvm#145538)

Tied Operands change is required for adding codegen patterns for
Qualcomm uC Xqcicm instructions
which will be done in a follow-up PR. This change leads to one of
instructions getting compressed even
when it shouldn't be. This case was not covered in llvm#143660. Added
changes to correctly handle this case.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend:RISC-V mc Machine (object) code tablegen
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants