[llvm] [TableGen] Fix minor index bugs in PseudoLoweringEmitter (PR #81605)

Alfie Richards via llvm-commits llvm-commits at lists.llvm.org
Tue Feb 13 05:41:20 PST 2024


https://github.com/AlfieRichardsArm updated https://github.com/llvm/llvm-project/pull/81605

>From 7aefdbeeb8745f4676625f1fe9104c7c76870a70 Mon Sep 17 00:00:00 2001
From: Alfie Richards <alfie.richards at arm.com>
Date: Tue, 13 Feb 2024 13:28:22 +0000
Subject: [PATCH] [TableGen] Fix minor index bugs in PseudoLoweringEmitter

This commit fixes some minor indices bugs in PseudoLoweringEmitter.cpp.
I don't believe these bugs would have had much of an effect previously
but they were causing some issues with some refactoring work with
".w" and ".n" operands I am working on currently.
---
 llvm/utils/TableGen/PseudoLoweringEmitter.cpp | 37 ++++++++++---------
 1 file changed, 20 insertions(+), 17 deletions(-)

diff --git a/llvm/utils/TableGen/PseudoLoweringEmitter.cpp b/llvm/utils/TableGen/PseudoLoweringEmitter.cpp
index 7f692f29192d9a..9c3e90e9066649 100644
--- a/llvm/utils/TableGen/PseudoLoweringEmitter.cpp
+++ b/llvm/utils/TableGen/PseudoLoweringEmitter.cpp
@@ -76,8 +76,8 @@ unsigned PseudoLoweringEmitter::addDagOperandMapping(
   unsigned OpsAdded = 0;
   for (unsigned i = 0, e = Dag->getNumArgs(); i != e; ++i) {
     if (DefInit *DI = dyn_cast<DefInit>(Dag->getArg(i))) {
-      // Physical register reference. Explicit check for the special case
-      // "zero_reg" definition.
+      // Physical register reference.
+      // Explicit check for the special case "zero_reg" definition.
       if (DI->getDef()->isSubClassOf("Register") ||
           DI->getDef()->getName() == "zero_reg") {
         OperandMap[BaseIdx + i].Kind = OpData::Reg;
@@ -88,23 +88,26 @@ unsigned PseudoLoweringEmitter::addDagOperandMapping(
 
       // Normal operands should always have the same type, or we have a
       // problem.
-      // FIXME: We probably shouldn't ever get a non-zero BaseIdx here.
-      assert(BaseIdx == 0 && "Named subargument in pseudo expansion?!");
+
       // FIXME: Are the message operand types backward?
-      if (DI->getDef() != Insn.Operands[BaseIdx + i].Rec) {
+      if (DI->getDef() != Insn.Operands[i].Rec) {
         PrintError(Rec, "In pseudo instruction '" + Rec->getName() +
                             "', operand type '" + DI->getDef()->getName() +
                             "' does not match expansion operand type '" +
-                            Insn.Operands[BaseIdx + i].Rec->getName() + "'");
+                            Insn.Operands[i].Rec->getName() + "'");
         PrintFatalNote(DI->getDef(),
                        "Value was assigned at the following location:");
       }
       // Source operand maps to destination operand. The Data element
       // will be filled in later, just set the Kind for now. Do it
       // for each corresponding MachineInstr operand, not just the first.
-      for (unsigned I = 0, E = Insn.Operands[i].MINumOperands; I != E; ++I)
+      for (unsigned I = 0, E = Insn.Operands[i].MINumOperands; I != E; ++I) {
         OperandMap[BaseIdx + i + I].Kind = OpData::Operand;
+      }
       OpsAdded += Insn.Operands[i].MINumOperands;
+      if (Insn.Operands[i].MINumOperands > 0)
+        BaseIdx += Insn.Operands[i].MINumOperands - 1;
+
     } else if (IntInit *II = dyn_cast<IntInit>(Dag->getArg(i))) {
       OperandMap[BaseIdx + i].Kind = OpData::Imm;
       OperandMap[BaseIdx + i].Data.Imm = II->getValue();
@@ -164,21 +167,21 @@ void PseudoLoweringEmitter::evaluateExpansion(Record *Rec) {
                    "Result was assigned at the following location:");
   }
 
-  if (Insn.Operands.size() != Dag->getNumArgs()) {
-    PrintError(Rec, "In pseudo instruction '" + Rec->getName() +
-                        "', result operator '" + Operator->getName() +
-                        "' has the wrong number of operands");
-    PrintFatalNote(Rec->getValue("ResultInst"),
-                   "Result was assigned at the following location:");
-  }
-
   unsigned NumMIOperands = 0;
   for (unsigned i = 0, e = Insn.Operands.size(); i != e; ++i)
     NumMIOperands += Insn.Operands[i].MINumOperands;
   IndexedMap<OpData> OperandMap;
   OperandMap.grow(NumMIOperands);
 
-  addDagOperandMapping(Rec, Dag, Insn, OperandMap, 0);
+  unsigned NumOps = addDagOperandMapping(Rec, Dag, Insn, OperandMap, 0);
+
+  if (NumOps < Dag->getNumArgs()) {
+    PrintError(Rec, "In pseudo instruction '" + Rec->getName() +
+                        "', result operator '" + Operator->getName() +
+                        "' has the wrong number of operands");
+    PrintFatalNote(Rec->getValue("ResultInst"),
+                   "Result was assigned at the following location:");
+  }
 
   // If there are more operands that weren't in the DAG, they have to
   // be operands that have default values, or we have an error. Currently,
@@ -196,7 +199,7 @@ void PseudoLoweringEmitter::evaluateExpansion(Record *Rec) {
     SourceOperands[SourceInsn.Operands[i].Name] = i;
 
   LLVM_DEBUG(dbgs() << "  Operand mapping:\n");
-  for (unsigned i = 0, e = Insn.Operands.size(); i != e; ++i) {
+  for (unsigned i = 0, e = Dag->getNumArgs(); i != e; ++i) {
     // We've already handled constant values. Just map instruction operands
     // here.
     if (OperandMap[Insn.Operands[i].MIOperandNo].Kind != OpData::Operand)



More information about the llvm-commits mailing list