Skip to content

Conversation

@topperc
Copy link
Collaborator

@topperc topperc commented Jul 24, 2025

This reduces the number of times we need to iterate over the operands.

Stacked on #150541

@llvmbot
Copy link
Member

llvmbot commented Jul 24, 2025

@llvm/pr-subscribers-tablegen

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

Author: Craig Topper (topperc)

Changes

This reduces the number of times we need to iterate over the operands.

Stacked on #150541


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

2 Files Affected:

  • (modified) llvm/lib/Target/RISCV/RISCVInstrInfoZc.td (+7-7)
  • (modified) llvm/utils/TableGen/CompressInstEmitter.cpp (+19-47)
diff --git a/llvm/lib/Target/RISCV/RISCVInstrInfoZc.td b/llvm/lib/Target/RISCV/RISCVInstrInfoZc.td
index f1734405fae63..ed1a60aa49cab 100644
--- a/llvm/lib/Target/RISCV/RISCVInstrInfoZc.td
+++ b/llvm/lib/Target/RISCV/RISCVInstrInfoZc.td
@@ -291,31 +291,31 @@ def : CompressPat<(MUL GPRC:$rs1, GPRC:$rs2, GPRC:$rs1),
 
 let Predicates = [HasStdExtZcb, HasStdExtZbb] in{
 def : CompressPat<(SEXT_B GPRC:$rs1, GPRC:$rs1),
-                  (C_SEXT_B GPRC:$rs1, GPRC:$rs1)>;
+                  (C_SEXT_B GPRC:$rs1)>;
 def : CompressPat<(SEXT_H GPRC:$rs1, GPRC:$rs1),
-                  (C_SEXT_H GPRC:$rs1, GPRC:$rs1)>;
+                  (C_SEXT_H GPRC:$rs1)>;
 } // Predicates = [HasStdExtZcb, HasStdExtZbb]
 
 let Predicates = [HasStdExtZcb, HasStdExtZbb] in{
 def : CompressPat<(ZEXT_H_RV32 GPRC:$rs1, GPRC:$rs1),
-                  (C_ZEXT_H GPRC:$rs1, GPRC:$rs1)>;
+                  (C_ZEXT_H GPRC:$rs1)>;
 def : CompressPat<(ZEXT_H_RV64 GPRC:$rs1, GPRC:$rs1),
-                  (C_ZEXT_H GPRC:$rs1, GPRC:$rs1)>;
+                  (C_ZEXT_H GPRC:$rs1)>;
 } // Predicates = [HasStdExtZcb, HasStdExtZbb]
 
 let Predicates = [HasStdExtZcb] in{
 def : CompressPat<(ANDI GPRC:$rs1, GPRC:$rs1, 255),
-                  (C_ZEXT_B GPRC:$rs1, GPRC:$rs1)>;
+                  (C_ZEXT_B GPRC:$rs1)>;
 } // Predicates = [HasStdExtZcb]
 
 let Predicates = [HasStdExtZcb, HasStdExtZba, IsRV64] in{
 def : CompressPat<(ADD_UW GPRC:$rs1, GPRC:$rs1, X0),
-                  (C_ZEXT_W GPRC:$rs1, GPRC:$rs1)>;
+                  (C_ZEXT_W GPRC:$rs1)>;
 } // Predicates = [HasStdExtZcb, HasStdExtZba, IsRV64]
 
 let Predicates = [HasStdExtZcb] in{
 def : CompressPat<(XORI GPRC:$rs1, GPRC:$rs1, -1),
-                  (C_NOT GPRC:$rs1, GPRC:$rs1)>;
+                  (C_NOT GPRC:$rs1)>;
 }
 
 let Predicates = [HasStdExtZcb] in{
diff --git a/llvm/utils/TableGen/CompressInstEmitter.cpp b/llvm/utils/TableGen/CompressInstEmitter.cpp
index afc892b068582..72e45ed5cca77 100644
--- a/llvm/utils/TableGen/CompressInstEmitter.cpp
+++ b/llvm/utils/TableGen/CompressInstEmitter.cpp
@@ -217,12 +217,8 @@ void CompressInstEmitter::addDagOperandMapping(const Record *Rec,
         Inst.Operands.back().MIOperandNo + Inst.Operands.back().MINumOperands;
   OperandMap.grow(NumMIOperands);
 
-  // TiedCount keeps track of the number of operands skipped in Inst
-  // operands list to get to the corresponding Dag operand. This is
-  // necessary because the number of operands in Inst might be greater
-  // than number of operands in the Dag due to how tied operands
-  // are represented.
-  unsigned TiedCount = 0;
+  // Tied operands are not represented in the DAG so we count them separately.
+  unsigned DAGOpNo = 0;
   unsigned OpNo = 0;
   for (const auto &Opnd : Inst.Operands) {
     int TiedOpIdx = Opnd.getTiedRegister();
@@ -231,15 +227,25 @@ void CompressInstEmitter::addDagOperandMapping(const Record *Rec,
       // Set the entry in OperandMap for the tied operand we're skipping.
       OperandMap[OpNo] = OperandMap[TiedOpIdx];
       ++OpNo;
-      ++TiedCount;
+
+      // Source instructions can have at most 1 tied operand.
+      if (IsSourceInst && (OpNo - DAGOpNo > 1))
+        PrintFatalError(Rec->getLoc(),
+                        "Input operands for Inst '" + Inst.TheDef->getName() +
+                            "' and input Dag operand count mismatch");
+
       continue;
     }
-    for (unsigned SubOp = 0; SubOp != Opnd.MINumOperands; ++SubOp, ++OpNo) {
-      unsigned DAGOpNo = OpNo - TiedCount;
+    for (unsigned SubOp = 0; SubOp != Opnd.MINumOperands;
+         ++SubOp, ++OpNo, ++DAGOpNo) {
       const Record *OpndRec = Opnd.Rec;
       if (Opnd.MINumOperands > 1)
         OpndRec = cast<DefInit>(Opnd.MIOperandInfo->getArg(SubOp))->getDef();
 
+      if (DAGOpNo >= Dag->getNumArgs())
+        PrintFatalError(Rec->getLoc(), "Inst '" + Inst.TheDef->getName() +
+                                           "' and Dag operand count mismatch");
+
       if (const auto *DI = dyn_cast<DefInit>(Dag->getArg(DAGOpNo))) {
         if (DI->getDef()->isSubClassOf("Register")) {
           // Check if the fixed register belongs to the Register class.
@@ -312,43 +318,11 @@ void CompressInstEmitter::addDagOperandMapping(const Record *Rec,
       Operands[ArgName] = {DAGOpNo, OpNo};
     }
   }
-}
 
-// Verify the Dag operand count is enough to build an instruction.
-static bool verifyDagOpCount(const CodeGenInstruction &Inst, const DagInit *Dag,
-                             bool IsSource) {
-  unsigned NumMIOperands = 0;
-
-  unsigned TiedOpCount = 0;
-  for (const auto &Op : Inst.Operands) {
-    NumMIOperands += Op.MINumOperands;
-    if (Op.getTiedRegister() != -1)
-      TiedOpCount++;
-  }
-
-  if (Dag->getNumArgs() == NumMIOperands)
-    return true;
-
-  // Source instructions are non compressed instructions and have at most one
-  // tied operand.
-  if (IsSource && (TiedOpCount > 1))
-    PrintFatalError(Inst.TheDef->getLoc(),
-                    "Input operands for Inst '" + Inst.TheDef->getName() +
-                        "' and input Dag operand count mismatch");
-
-  // The Dag can't have more arguments than the Instruction.
-  if (Dag->getNumArgs() > NumMIOperands)
-    PrintFatalError(Inst.TheDef->getLoc(),
-                    "Inst '" + Inst.TheDef->getName() +
-                        "' and Dag operand count mismatch");
-
-  // The Instruction might have tied operands so the Dag might have
-  // a fewer operand count.
-  if (Dag->getNumArgs() != (NumMIOperands - TiedOpCount))
-    PrintFatalError(Inst.TheDef->getLoc(),
-                    "Inst '" + Inst.TheDef->getName() +
-                        "' and Dag operand count mismatch");
-  return true;
+  // We shouldn't have extra Dag operands.
+  if (DAGOpNo != Dag->getNumArgs())
+    PrintFatalError(Rec->getLoc(), "Inst '" + Inst.TheDef->getName() +
+                                       "' and Dag operand count mismatch");
 }
 
 // Check that all names in the source DAG appear in the destionation DAG.
@@ -463,7 +437,6 @@ void CompressInstEmitter::evaluateCompressPat(const Record *Rec) {
   // Checking we are transforming from compressed to uncompressed instructions.
   const Record *SourceOperator = SourceDag->getOperatorAsDef(Rec->getLoc());
   CodeGenInstruction SourceInst(SourceOperator);
-  verifyDagOpCount(SourceInst, SourceDag, true);
 
   // Validate output Dag operands.
   const DagInit *DestDag = Rec->getValueAsDag("Output");
@@ -472,7 +445,6 @@ void CompressInstEmitter::evaluateCompressPat(const Record *Rec) {
 
   const Record *DestOperator = DestDag->getOperatorAsDef(Rec->getLoc());
   CodeGenInstruction DestInst(DestOperator);
-  verifyDagOpCount(DestInst, DestDag, false);
 
   if (SourceOperator->getValueAsInt("Size") <=
       DestOperator->getValueAsInt("Size"))

Copy link
Member

@lenary lenary left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks!

…nstEmitter.

This reduces the number of times we need to iterate over the
operands.
@topperc topperc force-pushed the pr/verifydagcount branch from 3cee18f to 8857dce Compare July 25, 2025 01:07
@topperc topperc merged commit 076d305 into llvm:main Jul 25, 2025
8 checks passed
@topperc topperc deleted the pr/verifydagcount branch July 25, 2025 02:50
mahesh-attarde pushed a commit to mahesh-attarde/llvm-project that referenced this pull request Jul 28, 2025
…nstEmitter. (llvm#150548)

This reduces the number of times we need to iterate over the operands.
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