[llvm] [TableGen] More generically handle tied source operands in CompressInstEmitter. (PR #146183)
Craig Topper via llvm-commits
llvm-commits at lists.llvm.org
Tue Jul 1 21:42:27 PDT 2025
https://github.com/topperc updated https://github.com/llvm/llvm-project/pull/146183
>From 6d74b3a492a4a0fbe5c168f4352fd1c2038c8496 Mon Sep 17 00:00:00 2001
From: Craig Topper <craig.topper at sifive.com>
Date: Fri, 27 Jun 2025 16:02:41 -0700
Subject: [PATCH 1/4] [TableGen] Try to more generically handle tied source
operands in CompressInstEmitter.
Move the creation of OperandMap from createDagOperandMapping to
the loop in addDagOperandMapping. Expand it to store the DAG operand
number and the MI operand number which will be different when
there are tied operands.
Rename createDagOperandMapping to checkDagOperandMapping to better
describe the remaining code.
I didn't lift the restriction that a source instruction can only
have one tied operand, but we should be able to if we have a use case.
There's a slight difference in the generate output. We now
check that operand 0 and 2 of QC_MVEQI are equal instead of
operand 1 and 2. This should be equivalent since operand 0 and 1
have a tied constraint.
---
llvm/utils/TableGen/CompressInstEmitter.cpp | 158 +++++++++-----------
1 file changed, 69 insertions(+), 89 deletions(-)
diff --git a/llvm/utils/TableGen/CompressInstEmitter.cpp b/llvm/utils/TableGen/CompressInstEmitter.cpp
index d3e98b3096afe..3758814d7c07b 100644
--- a/llvm/utils/TableGen/CompressInstEmitter.cpp
+++ b/llvm/utils/TableGen/CompressInstEmitter.cpp
@@ -126,27 +126,24 @@ class CompressInstEmitter {
std::vector<CompressPat> CompressPatterns;
void addDagOperandMapping(const Record *Rec, const DagInit *Dag,
const CodeGenInstruction &Inst,
- IndexedMap<OpData> &OperandMap, bool IsSourceInst,
- unsigned *SourceLastTiedOpPtr);
+ IndexedMap<OpData> &OperandMap,
+ StringMap<std::pair<unsigned, unsigned>> &Operands,
+ bool IsSourceInst);
void evaluateCompressPat(const Record *Compress);
void emitCompressInstEmitter(raw_ostream &OS, EmitterType EType);
bool validateTypes(const Record *DagOpType, const Record *InstOpType,
bool IsSourceInst);
bool validateRegister(const Record *Reg, const Record *RegClass);
- void createDagOperandMapping(const Record *Rec,
- StringMap<unsigned> &SourceOperands,
- StringMap<unsigned> &DestOperands,
- const DagInit *SourceDag, const DagInit *DestDag,
- IndexedMap<OpData> &SourceOperandMap,
- bool HasSourceTiedOp);
-
- void createInstOperandMapping(const Record *Rec, const DagInit *SourceDag,
- const DagInit *DestDag,
- IndexedMap<OpData> &SourceOperandMap,
- IndexedMap<OpData> &DestOperandMap,
- StringMap<unsigned> &SourceOperands,
- const CodeGenInstruction &DestInst,
- unsigned SourceLastTiedOp);
+ void checkDagOperandMapping(
+ const Record *Rec,
+ const StringMap<std::pair<unsigned, unsigned>> &DestOperands,
+ const DagInit *SourceDag, const DagInit *DestDag);
+
+ void createInstOperandMapping(
+ const Record *Rec, const DagInit *SourceDag, const DagInit *DestDag,
+ IndexedMap<OpData> &SourceOperandMap, IndexedMap<OpData> &DestOperandMap,
+ StringMap<std::pair<unsigned, unsigned>> &SourceOperands,
+ const CodeGenInstruction &DestInst);
public:
CompressInstEmitter(const RecordKeeper &R) : Records(R), Target(R) {}
@@ -198,6 +195,10 @@ bool CompressInstEmitter::validateTypes(const Record *DagOpType,
return true;
}
+static bool validateArgsTypes(const Init *Arg1, const Init *Arg2) {
+ return cast<DefInit>(Arg1)->getDef() == cast<DefInit>(Arg2)->getDef();
+}
+
/// The patterns in the Dag contain different types of operands:
/// Register operands, e.g.: GPRC:$rs1; Fixed registers, e.g: X1; Immediate
/// operands, e.g.: simm6:$imm; Fixed immediate operands, e.g.: 0. This function
@@ -205,12 +206,10 @@ bool CompressInstEmitter::validateTypes(const Record *DagOpType,
/// operands and fixed registers it expects the Dag operand type to be contained
/// in the instantiated instruction operand type. For immediate operands and
/// immediates no validation checks are enforced at pattern validation time.
-void CompressInstEmitter::addDagOperandMapping(const Record *Rec,
- const DagInit *Dag,
- const CodeGenInstruction &Inst,
- IndexedMap<OpData> &OperandMap,
- bool IsSourceInst,
- unsigned *SourceLastTiedOpPtr) {
+void CompressInstEmitter::addDagOperandMapping(
+ const Record *Rec, const DagInit *Dag, const CodeGenInstruction &Inst,
+ IndexedMap<OpData> &OperandMap,
+ StringMap<std::pair<unsigned, unsigned>> &Operands, bool IsSourceInst) {
unsigned NumMIOperands = 0;
if (!Inst.Operands.empty())
NumMIOperands =
@@ -224,16 +223,12 @@ void CompressInstEmitter::addDagOperandMapping(const Record *Rec,
// are represented.
unsigned TiedCount = 0;
unsigned OpNo = 0;
- if (IsSourceInst)
- *SourceLastTiedOpPtr = std::numeric_limits<unsigned int>::max();
for (const auto &Opnd : Inst.Operands) {
int TiedOpIdx = Opnd.getTiedRegister();
if (-1 != TiedOpIdx) {
assert((unsigned)TiedOpIdx < OpNo);
// Set the entry in OperandMap for the tied operand we're skipping.
OperandMap[OpNo] = OperandMap[TiedOpIdx];
- if (IsSourceInst)
- *SourceLastTiedOpPtr = OpNo;
++OpNo;
++TiedCount;
continue;
@@ -290,6 +285,28 @@ void CompressInstEmitter::addDagOperandMapping(const Record *Rec,
} else {
llvm_unreachable("Unhandled CompressPat argument type!");
}
+
+ // Create a mapping between the operand name in the Dag (e.g. $rs1) and
+ // its index in the list of Dag operands and check that operands with the
+ // same name have the same type. For example in 'C_ADD $rs1, $rs2' we
+ // generate the mapping $rs1 --> 0, $rs2 ---> 1. If the operand appears
+ // twice in the (tied) same Dag we use the last occurrence for indexing.
+ if (Dag->getArgNameStr(DAGOpNo).empty())
+ continue;
+
+ if (IsSourceInst) {
+ auto It = Operands.find(Dag->getArgNameStr(DAGOpNo));
+ if (It != Operands.end()) {
+ OperandMap[OpNo].TiedOpIdx = It->getValue().second;
+ if (!validateArgsTypes(Dag->getArg(It->getValue().first),
+ Dag->getArg(DAGOpNo)))
+ PrintFatalError(Rec->getLoc(),
+ "Input Operand '" + Dag->getArgNameStr(DAGOpNo) +
+ "' has a mismatched tied operand!\n");
+ }
+ }
+
+ Operands[Dag->getArgNameStr(DAGOpNo)] = {DAGOpNo, OpNo};
}
}
}
@@ -331,60 +348,29 @@ static bool verifyDagOpCount(const CodeGenInstruction &Inst, const DagInit *Dag,
return true;
}
-static bool validateArgsTypes(const Init *Arg1, const Init *Arg2) {
- return cast<DefInit>(Arg1)->getDef() == cast<DefInit>(Arg2)->getDef();
-}
-
-// Creates a mapping between the operand name in the Dag (e.g. $rs1) and
-// its index in the list of Dag operands and checks that operands with the same
-// name have the same types. For example in 'C_ADD $rs1, $rs2' we generate the
-// mapping $rs1 --> 0, $rs2 ---> 1. If the operand appears twice in the (tied)
-// same Dag we use the last occurrence for indexing.
-void CompressInstEmitter::createDagOperandMapping(
- const Record *Rec, StringMap<unsigned> &SourceOperands,
- StringMap<unsigned> &DestOperands, const DagInit *SourceDag,
- 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.
- if ("" == DestDag->getArgNameStr(I))
- continue;
- DestOperands[DestDag->getArgNameStr(I)] = I;
- }
+// Check that all names in the source DAG appear in the destionation DAG.
+void CompressInstEmitter::checkDagOperandMapping(
+ const Record *Rec,
+ const StringMap<std::pair<unsigned, unsigned>> &DestOperands,
+ const DagInit *SourceDag, const DagInit *DestDag) {
for (unsigned I = 0; I < SourceDag->getNumArgs(); ++I) {
// Skip fixed immediates and registers, they were handled in
// addDagOperandMapping.
- if ("" == SourceDag->getArgNameStr(I))
+ if (SourceDag->getArgNameStr(I).empty())
continue;
- StringMap<unsigned>::iterator It =
- SourceOperands.find(SourceDag->getArgNameStr(I));
- if (It != SourceOperands.end()) {
- // Operand sharing the same name in the Dag should be mapped as tied.
- 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(),
- "Input Operand '" + SourceDag->getArgNameStr(I) +
- "' has a mismatched tied operand!\n");
- }
- It = DestOperands.find(SourceDag->getArgNameStr(I));
+ auto It = DestOperands.find(SourceDag->getArgNameStr(I));
if (It == DestOperands.end())
PrintFatalError(Rec->getLoc(), "Operand " + SourceDag->getArgNameStr(I) +
" defined in Input Dag but not used in"
" Output Dag!\n");
// Input Dag operand types must match output Dag operand type.
- if (!validateArgsTypes(DestDag->getArg(It->getValue()),
+ if (!validateArgsTypes(DestDag->getArg(It->getValue().first),
SourceDag->getArg(I)))
PrintFatalError(Rec->getLoc(), "Type mismatch between Input and "
"Output Dag operand '" +
SourceDag->getArgNameStr(I) + "'!");
- SourceOperands[SourceDag->getArgNameStr(I)] = I;
}
}
@@ -394,8 +380,8 @@ void CompressInstEmitter::createDagOperandMapping(
void CompressInstEmitter::createInstOperandMapping(
const Record *Rec, const DagInit *SourceDag, const DagInit *DestDag,
IndexedMap<OpData> &SourceOperandMap, IndexedMap<OpData> &DestOperandMap,
- StringMap<unsigned> &SourceOperands, const CodeGenInstruction &DestInst,
- unsigned SourceLastTiedOp) {
+ StringMap<std::pair<unsigned, unsigned>> &SourceOperands,
+ const CodeGenInstruction &DestInst) {
// TiedCount keeps track of the number of operands skipped in Inst
// operands list to get to the corresponding Dag operand.
unsigned TiedCount = 0;
@@ -425,8 +411,7 @@ void CompressInstEmitter::createInstOperandMapping(
continue;
unsigned DagArgIdx = OpNo - TiedCount;
- StringMap<unsigned>::iterator SourceOp =
- SourceOperands.find(DestDag->getArgNameStr(DagArgIdx));
+ auto SourceOp = SourceOperands.find(DestDag->getArgNameStr(DagArgIdx));
if (SourceOp == SourceOperands.end())
PrintFatalError(Rec->getLoc(),
"Output Dag operand '" +
@@ -434,7 +419,7 @@ void CompressInstEmitter::createInstOperandMapping(
"' has no matching input Dag operand.");
assert(DestDag->getArgNameStr(DagArgIdx) ==
- SourceDag->getArgNameStr(SourceOp->getValue()) &&
+ SourceDag->getArgNameStr(SourceOp->getValue().first) &&
"Incorrect operand mapping detected!\n");
// Following four lines ensure the correct handling of a single tied
@@ -442,12 +427,10 @@ void CompressInstEmitter::createInstOperandMapping(
// appropriate Dag argument which is not correct in presence of tied
// operand in the Source Inst and must be incremented by 1 to reflect
// correct position of the operand in Source Inst
- unsigned SourceDagOp = SourceOp->getValue();
- if (SourceDagOp >= SourceLastTiedOp)
- SourceDagOp++;
- DestOperandMap[OpNo].Data.Operand = SourceDagOp;
- SourceOperandMap[SourceDagOp].Data.Operand = OpNo;
- LLVM_DEBUG(dbgs() << " " << SourceDagOp << " ====> " << OpNo << "\n");
+ unsigned SourceOpNo = SourceOp->getValue().second;
+ DestOperandMap[OpNo].Data.Operand = SourceOpNo;
+ SourceOperandMap[SourceOpNo].Data.Operand = OpNo;
+ LLVM_DEBUG(dbgs() << " " << SourceOpNo << " ====> " << OpNo << "\n");
}
}
}
@@ -506,27 +489,24 @@ void CompressInstEmitter::evaluateCompressPat(const Record *Rec) {
// Fill the mapping from the source to destination instructions.
IndexedMap<OpData> SourceOperandMap;
- unsigned SourceLastTiedOp; // postion of the last tied operand in Source Inst
+ // Map from arg name to DAG operand number and MI operand number.
+ StringMap<std::pair<unsigned, unsigned>> SourceOperands;
// Create a mapping between source Dag operands and source Inst operands.
addDagOperandMapping(Rec, SourceDag, SourceInst, SourceOperandMap,
- /*IsSourceInst*/ true, &SourceLastTiedOp);
+ SourceOperands, /*IsSourceInst*/ true);
IndexedMap<OpData> DestOperandMap;
+ // Map from arg name to DAG operand number and MI operand number.
+ StringMap<std::pair<unsigned, unsigned>> DestOperands;
// Create a mapping between destination Dag operands and destination Inst
// operands.
- addDagOperandMapping(Rec, DestDag, DestInst, DestOperandMap,
- /*IsSourceInst*/ false, nullptr);
-
- StringMap<unsigned> SourceOperands;
- StringMap<unsigned> DestOperands;
- bool HasSourceTiedOp =
- SourceLastTiedOp != std::numeric_limits<unsigned int>::max();
- createDagOperandMapping(Rec, SourceOperands, DestOperands, SourceDag, DestDag,
- SourceOperandMap, HasSourceTiedOp);
+ addDagOperandMapping(Rec, DestDag, DestInst, DestOperandMap, DestOperands,
+ /*IsSourceInst*/ false);
+
+ checkDagOperandMapping(Rec, DestOperands, SourceDag, DestDag);
// Create operand mapping between the source and destination instructions.
createInstOperandMapping(Rec, SourceDag, DestDag, SourceOperandMap,
- DestOperandMap, SourceOperands, DestInst,
- SourceLastTiedOp);
+ DestOperandMap, SourceOperands, DestInst);
// Get the target features for the CompressPat.
std::vector<const Record *> PatReqFeatures;
>From 255dfa0399c57239ce1e98375d241f70e9b745b6 Mon Sep 17 00:00:00 2001
From: Craig Topper <craig.topper at sifive.com>
Date: Tue, 1 Jul 2025 13:34:23 -0700
Subject: [PATCH 2/4] fixup! address review comments
---
llvm/utils/TableGen/CompressInstEmitter.cpp | 38 +++++++++++----------
1 file changed, 20 insertions(+), 18 deletions(-)
diff --git a/llvm/utils/TableGen/CompressInstEmitter.cpp b/llvm/utils/TableGen/CompressInstEmitter.cpp
index 3758814d7c07b..23771255a766c 100644
--- a/llvm/utils/TableGen/CompressInstEmitter.cpp
+++ b/llvm/utils/TableGen/CompressInstEmitter.cpp
@@ -290,23 +290,25 @@ void CompressInstEmitter::addDagOperandMapping(
// its index in the list of Dag operands and check that operands with the
// same name have the same type. For example in 'C_ADD $rs1, $rs2' we
// generate the mapping $rs1 --> 0, $rs2 ---> 1. If the operand appears
- // twice in the (tied) same Dag we use the last occurrence for indexing.
- if (Dag->getArgNameStr(DAGOpNo).empty())
+ // twice in the same Dag (tied in the compressed instruction), we note
+ // the previous index in the TiedOpIdx field.
+ StringRef ArgName = Dag->getArgNameStr(DAGOpNo);
+ if (ArgName.empty())
continue;
if (IsSourceInst) {
- auto It = Operands.find(Dag->getArgNameStr(DAGOpNo));
+ auto It = Operands.find(ArgName);
if (It != Operands.end()) {
OperandMap[OpNo].TiedOpIdx = It->getValue().second;
if (!validateArgsTypes(Dag->getArg(It->getValue().first),
Dag->getArg(DAGOpNo)))
PrintFatalError(Rec->getLoc(),
- "Input Operand '" + Dag->getArgNameStr(DAGOpNo) +
- "' has a mismatched tied operand!\n");
+ "Input Operand '" + ArgName +
+ "' has a mismatched tied operand!");
}
}
- Operands[Dag->getArgNameStr(DAGOpNo)] = {DAGOpNo, OpNo};
+ Operands[ArgName] = {DAGOpNo, OpNo};
}
}
}
@@ -357,20 +359,21 @@ void CompressInstEmitter::checkDagOperandMapping(
for (unsigned I = 0; I < SourceDag->getNumArgs(); ++I) {
// Skip fixed immediates and registers, they were handled in
// addDagOperandMapping.
- if (SourceDag->getArgNameStr(I).empty())
+ StringRef SourceDag->getArgNameStr(I);
+ if (ArgName.empty())
continue;
- auto It = DestOperands.find(SourceDag->getArgNameStr(I));
+ auto It = DestOperands.find(ArgName);
if (It == DestOperands.end())
- PrintFatalError(Rec->getLoc(), "Operand " + SourceDag->getArgNameStr(I) +
+ PrintFatalError(Rec->getLoc(), "Operand " + ArgName +
" defined in Input Dag but not used in"
- " Output Dag!\n");
+ " Output Dag!");
// Input Dag operand types must match output Dag operand type.
if (!validateArgsTypes(DestDag->getArg(It->getValue().first),
SourceDag->getArg(I)))
PrintFatalError(Rec->getLoc(), "Type mismatch between Input and "
"Output Dag operand '" +
- SourceDag->getArgNameStr(I) + "'!");
+ ArgName + "'!");
}
}
@@ -411,15 +414,14 @@ void CompressInstEmitter::createInstOperandMapping(
continue;
unsigned DagArgIdx = OpNo - TiedCount;
- auto SourceOp = SourceOperands.find(DestDag->getArgNameStr(DagArgIdx));
+ StringRef ArgName = DestDag->getArgNameStr(DagArgIdx);
+ auto SourceOp = SourceOperands.find(ArgName);
if (SourceOp == SourceOperands.end())
PrintFatalError(Rec->getLoc(),
- "Output Dag operand '" +
- DestDag->getArgNameStr(DagArgIdx) +
+ "Output Dag operand '" + ArgName +
"' has no matching input Dag operand.");
- assert(DestDag->getArgNameStr(DagArgIdx) ==
- SourceDag->getArgNameStr(SourceOp->getValue().first) &&
+ assert(ArgName == SourceDag->getArgNameStr(SourceOp->getValue().first) &&
"Incorrect operand mapping detected!\n");
// Following four lines ensure the correct handling of a single tied
@@ -604,7 +606,7 @@ void CompressInstEmitter::emitCompressInstEmitter(raw_ostream &OS,
if (!AsmWriter->getValueAsInt("PassSubtarget"))
PrintFatalError(AsmWriter->getLoc(),
"'PassSubtarget' is false. SubTargetInfo object is needed "
- "for target features.\n");
+ "for target features.");
StringRef TargetName = Target.getName();
@@ -760,7 +762,7 @@ void CompressInstEmitter::emitCompressInstEmitter(raw_ostream &OS,
<< ").getReg() == MI.getOperand("
<< SourceOperandMap[OpNo].TiedOpIdx << ").getReg()) &&\n";
else
- PrintFatalError("Unexpected tied operand types!\n");
+ PrintFatalError("Unexpected tied operand types!");
}
for (unsigned SubOp = 0; SubOp != SourceOperand.MINumOperands; ++SubOp) {
// Check for fixed immediates\registers in the source instruction.
>From 89cac8b5747d99aea38291cb1810afaf4385dcbf Mon Sep 17 00:00:00 2001
From: Craig Topper <craig.topper at sifive.com>
Date: Tue, 1 Jul 2025 13:59:01 -0700
Subject: [PATCH 3/4] fixup! fix build error
---
llvm/utils/TableGen/CompressInstEmitter.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/llvm/utils/TableGen/CompressInstEmitter.cpp b/llvm/utils/TableGen/CompressInstEmitter.cpp
index 23771255a766c..01fbc5f85f254 100644
--- a/llvm/utils/TableGen/CompressInstEmitter.cpp
+++ b/llvm/utils/TableGen/CompressInstEmitter.cpp
@@ -359,7 +359,7 @@ void CompressInstEmitter::checkDagOperandMapping(
for (unsigned I = 0; I < SourceDag->getNumArgs(); ++I) {
// Skip fixed immediates and registers, they were handled in
// addDagOperandMapping.
- StringRef SourceDag->getArgNameStr(I);
+ StringRef ArgName = SourceDag->getArgNameStr(I);
if (ArgName.empty())
continue;
>From d44d939c9ea0ae62cced1cf3c91073c026d8cd86 Mon Sep 17 00:00:00 2001
From: Craig Topper <craig.topper at sifive.com>
Date: Tue, 1 Jul 2025 21:42:09 -0700
Subject: [PATCH 4/4] fixup! Remove state comment.
---
llvm/utils/TableGen/CompressInstEmitter.cpp | 5 -----
1 file changed, 5 deletions(-)
diff --git a/llvm/utils/TableGen/CompressInstEmitter.cpp b/llvm/utils/TableGen/CompressInstEmitter.cpp
index 01fbc5f85f254..a10ca490583a9 100644
--- a/llvm/utils/TableGen/CompressInstEmitter.cpp
+++ b/llvm/utils/TableGen/CompressInstEmitter.cpp
@@ -424,11 +424,6 @@ void CompressInstEmitter::createInstOperandMapping(
assert(ArgName == SourceDag->getArgNameStr(SourceOp->getValue().first) &&
"Incorrect operand mapping detected!\n");
- // Following four lines ensure the correct handling of a single tied
- // operand in the Source Inst. SourceDagOp points to the position of
- // appropriate Dag argument which is not correct in presence of tied
- // operand in the Source Inst and must be incremented by 1 to reflect
- // correct position of the operand in Source Inst
unsigned SourceOpNo = SourceOp->getValue().second;
DestOperandMap[OpNo].Data.Operand = SourceOpNo;
SourceOperandMap[SourceOpNo].Data.Operand = OpNo;
More information about the llvm-commits
mailing list