Skip to content

Commit 18397f6

Browse files
topperctopperc
authored
[RISCV] Use Record from CompressPat in compress/uncompress functions. (#150664)
Instead of using the Record from the instruction definition, use the Record specified in the CompressPat DAG. This will allow us to use Records that are subsets of both the source and destination. I want to use this to merge the C_*_HINT instructions back into their regular non-HINT versions, but prevent those encodings from being part of compress/uncompress. For example, we will use GPRNoX0 for the C_ADDI CompressPat while both C_ADDI and ADDI will use GPR in their instruction definitions. To do this I've recorded the original DAG Record in the OperandMap using a struct in the union to represent the 3 fields needed for an Operand. Previously we stored TiedOpIdx outside the union, but only used it for Operand. There is a verification hole here where we don't have any way to check that an immediate predicate is a subset of an instruction predicate at tablegen time. Prior to #148660 we had this hole in one direction, but that patch made it in two directions. I'm not sure if this patch makes it any worse. Now we're using what is in the CompressPat where before we were using whatever was in the instructions and ignoring the predicate in the CompressPat.
1 parent 9e09c4d commit 18397f6

File tree

2 files changed

+65
-53
lines changed

2 files changed

+65
-53
lines changed

‎llvm/test/TableGen/CompressInstEmitter/suboperands.td‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -161,15 +161,15 @@ def : CompressPat<(BigInst3 RegsC:$dst, RegsC:$src, simm6:$imm),
161161
// CHECK-NEXT: } // if
162162

163163
// CHECK-LABEL: ArchValidateMCOperandForUncompress
164-
// CHECK: // simm12
165-
// CHECK: return isInt<12>(Imm);
164+
// CHECK: // simm6
165+
// CHECK: return isInt<6>(Imm);
166166

167167
// CHECK-LABEL: uncompressInst
168168
// CHECK: case Arch::SmallInst:
169169
// CHECK-NEXT: if (MI.getOperand(0).isReg() &&
170-
// CHECK-NEXT: ArchMCRegisterClasses[Arch::RegsRegClassID].contains(MI.getOperand(0).getReg()) &&
170+
// CHECK-NEXT: ArchMCRegisterClasses[Arch::RegsCRegClassID].contains(MI.getOperand(0).getReg()) &&
171171
// CHECK-NEXT: MI.getOperand(1).isReg() &&
172-
// CHECK-NEXT: ArchMCRegisterClasses[Arch::RegsRegClassID].contains(MI.getOperand(1).getReg()) &&
172+
// CHECK-NEXT: ArchMCRegisterClasses[Arch::RegsCRegClassID].contains(MI.getOperand(1).getReg()) &&
173173
// CHECK-NEXT: ArchValidateMCOperandForUncompress(MI.getOperand(2), STI, 1)) {
174174
// CHECK-NEXT: // big $dst, $addr
175175
// CHECK-NEXT: OutInst.setOpcode(Arch::BigInst);
@@ -183,9 +183,9 @@ def : CompressPat<(BigInst3 RegsC:$dst, RegsC:$src, simm6:$imm),
183183
// CHECK-NEXT: } // if
184184
// CHECK: case Arch::SmallInst2:
185185
// CHECK-NEXT: if (MI.getOperand(0).isReg() &&
186-
// CHECK-NEXT: ArchMCRegisterClasses[Arch::RegsRegClassID].contains(MI.getOperand(0).getReg()) &&
186+
// CHECK-NEXT: ArchMCRegisterClasses[Arch::RegsCRegClassID].contains(MI.getOperand(0).getReg()) &&
187187
// CHECK-NEXT: MI.getOperand(1).isReg() &&
188-
// CHECK-NEXT: ArchMCRegisterClasses[Arch::RegsRegClassID].contains(MI.getOperand(1).getReg()) &&
188+
// CHECK-NEXT: ArchMCRegisterClasses[Arch::RegsCRegClassID].contains(MI.getOperand(1).getReg()) &&
189189
// CHECK-NEXT: ArchValidateMCOperandForUncompress(MI.getOperand(2), STI, 1)) {
190190
// CHECK-NEXT: // big $dst, $addr
191191
// CHECK-NEXT: OutInst.setOpcode(Arch::BigInst2);
@@ -199,9 +199,9 @@ def : CompressPat<(BigInst3 RegsC:$dst, RegsC:$src, simm6:$imm),
199199
// CHECK-NEXT: } // if
200200
// CHECK: case Arch::SmallInst3:
201201
// CHECK-NEXT: if (MI.getOperand(0).isReg() &&
202-
// CHECK-NEXT: ArchMCRegisterClasses[Arch::RegsRegClassID].contains(MI.getOperand(0).getReg()) &&
202+
// CHECK-NEXT: ArchMCRegisterClasses[Arch::RegsCRegClassID].contains(MI.getOperand(0).getReg()) &&
203203
// CHECK-NEXT: MI.getOperand(1).isReg() &&
204-
// CHECK-NEXT: ArchMCRegisterClasses[Arch::RegsRegClassID].contains(MI.getOperand(1).getReg()) &&
204+
// CHECK-NEXT: ArchMCRegisterClasses[Arch::RegsCRegClassID].contains(MI.getOperand(1).getReg()) &&
205205
// CHECK-NEXT: ArchValidateMCOperandForUncompress(MI.getOperand(2), STI, 1)) {
206206
// CHECK-NEXT: // big $dst, $src, $imm
207207
// CHECK-NEXT: OutInst.setOpcode(Arch::BigInst3);

‎llvm/utils/TableGen/CompressInstEmitter.cpp‎

Lines changed: 57 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -86,16 +86,22 @@ namespace {
8686
class CompressInstEmitter {
8787
struct OpData {
8888
enum MapKind { Operand, Imm, Reg } Kind;
89-
union {
89+
// Info for an operand.
90+
struct OpndInfo {
91+
// Record from the Dag.
92+
const Record *DagRec;
9093
// Operand number mapped to.
91-
unsigned OpNo;
94+
unsigned Idx;
95+
// Tied operand index within the instruction.
96+
int TiedOpIdx;
97+
};
98+
union {
99+
OpndInfo OpInfo;
92100
// Integer immediate value.
93101
int64_t ImmVal;
94102
// Physical register.
95103
const Record *RegRec;
96104
};
97-
// Tied operand index within the instruction.
98-
int TiedOpIdx = -1;
99105
};
100106
struct ArgData {
101107
unsigned DAGOpNo;
@@ -273,6 +279,31 @@ void CompressInstEmitter::addDagOperandMapping(const Record *Rec,
273279
"' in the corresponding instruction operand!");
274280

275281
OperandMap[OpNo].Kind = OpData::Operand;
282+
OperandMap[OpNo].OpInfo.DagRec = DI->getDef();
283+
OperandMap[OpNo].OpInfo.TiedOpIdx = -1;
284+
285+
// Create a mapping between the operand name in the Dag (e.g. $rs1) and
286+
// its index in the list of Dag operands and check that operands with
287+
// the same name have the same type. For example in 'C_ADD $rs1, $rs2'
288+
// we generate the mapping $rs1 --> 0, $rs2 ---> 1. If the operand
289+
// appears twice in the same Dag (tied in the compressed instruction),
290+
// we note the previous index in the TiedOpIdx field.
291+
StringRef ArgName = Dag->getArgNameStr(DAGOpNo);
292+
if (ArgName.empty())
293+
continue;
294+
295+
if (IsSourceInst) {
296+
auto It = Operands.find(ArgName);
297+
if (It != Operands.end()) {
298+
OperandMap[OpNo].OpInfo.TiedOpIdx = It->getValue().MIOpNo;
299+
if (OperandMap[It->getValue().MIOpNo].OpInfo.DagRec != DI->getDef())
300+
PrintFatalError(Rec->getLoc(),
301+
"Input Operand '" + ArgName +
302+
"' has a mismatched tied operand!");
303+
}
304+
}
305+
306+
Operands[ArgName] = {DAGOpNo, OpNo};
276307
} else if (const auto *II = dyn_cast<IntInit>(Dag->getArg(DAGOpNo))) {
277308
// Validate that corresponding instruction operand expects an immediate.
278309
if (!OpndRec->isSubClassOf("Operand"))
@@ -292,30 +323,6 @@ void CompressInstEmitter::addDagOperandMapping(const Record *Rec,
292323
} else {
293324
llvm_unreachable("Unhandled CompressPat argument type!");
294325
}
295-
296-
// Create a mapping between the operand name in the Dag (e.g. $rs1) and
297-
// its index in the list of Dag operands and check that operands with the
298-
// same name have the same type. For example in 'C_ADD $rs1, $rs2' we
299-
// generate the mapping $rs1 --> 0, $rs2 ---> 1. If the operand appears
300-
// twice in the same Dag (tied in the compressed instruction), we note
301-
// the previous index in the TiedOpIdx field.
302-
StringRef ArgName = Dag->getArgNameStr(DAGOpNo);
303-
if (ArgName.empty())
304-
continue;
305-
306-
if (IsSourceInst) {
307-
auto It = Operands.find(ArgName);
308-
if (It != Operands.end()) {
309-
OperandMap[OpNo].TiedOpIdx = It->getValue().MIOpNo;
310-
if (!validateArgsTypes(Dag->getArg(It->getValue().DAGOpNo),
311-
Dag->getArg(DAGOpNo)))
312-
PrintFatalError(Rec->getLoc(),
313-
"Input Operand '" + ArgName +
314-
"' has a mismatched tied operand!");
315-
}
316-
}
317-
318-
Operands[ArgName] = {DAGOpNo, OpNo};
319326
}
320327
}
321328

@@ -372,8 +379,9 @@ void CompressInstEmitter::createInstOperandMapping(
372379
if (DestOperandMap[OpNo].Kind == OpData::Operand)
373380
// No need to fill the SourceOperandMap here since it was mapped to
374381
// destination operand 'TiedInstOpIdx' in a previous iteration.
375-
LLVM_DEBUG(dbgs() << " " << DestOperandMap[OpNo].OpNo << " ====> "
376-
<< OpNo << " Dest operand tied with operand '"
382+
LLVM_DEBUG(dbgs() << " " << DestOperandMap[OpNo].OpInfo.Idx
383+
<< " ====> " << OpNo
384+
<< " Dest operand tied with operand '"
377385
<< TiedInstOpIdx << "'\n");
378386
++OpNo;
379387
continue;
@@ -398,8 +406,8 @@ void CompressInstEmitter::createInstOperandMapping(
398406
"Incorrect operand mapping detected!\n");
399407

400408
unsigned SourceOpNo = SourceOp->getValue().MIOpNo;
401-
DestOperandMap[OpNo].OpNo = SourceOpNo;
402-
SourceOperandMap[SourceOpNo].OpNo = OpNo;
409+
DestOperandMap[OpNo].OpInfo.Idx = SourceOpNo;
410+
SourceOperandMap[SourceOpNo].OpInfo.Idx = OpNo;
403411
LLVM_DEBUG(dbgs() << " " << SourceOpNo << " ====> " << OpNo << "\n");
404412
}
405413
}
@@ -721,21 +729,24 @@ void CompressInstEmitter::emitCompressInstEmitter(raw_ostream &OS,
721729
// Start Source Inst operands validation.
722730
unsigned OpNo = 0;
723731
for (const auto &SourceOperand : Source.Operands) {
724-
if (SourceOperandMap[OpNo].TiedOpIdx != -1) {
725-
if (Source.Operands[OpNo].Rec->isSubClassOf("RegisterClass"))
726-
CondStream.indent(8)
727-
<< "(MI.getOperand(" << OpNo << ").isReg()) && (MI.getOperand("
728-
<< SourceOperandMap[OpNo].TiedOpIdx << ").isReg()) &&\n"
729-
<< indent(8) << "(MI.getOperand(" << OpNo
730-
<< ").getReg() == MI.getOperand("
731-
<< SourceOperandMap[OpNo].TiedOpIdx << ").getReg()) &&\n";
732-
else
733-
PrintFatalError("Unexpected tied operand types!");
734-
}
735732
for (unsigned SubOp = 0; SubOp != SourceOperand.MINumOperands; ++SubOp) {
736733
// Check for fixed immediates\registers in the source instruction.
737734
switch (SourceOperandMap[OpNo].Kind) {
738735
case OpData::Operand:
736+
if (SourceOperandMap[OpNo].OpInfo.TiedOpIdx != -1) {
737+
if (Source.Operands[OpNo].Rec->isSubClassOf("RegisterClass"))
738+
CondStream.indent(8) << "(MI.getOperand(" << OpNo
739+
<< ").isReg()) && (MI.getOperand("
740+
<< SourceOperandMap[OpNo].OpInfo.TiedOpIdx
741+
<< ").isReg()) &&\n"
742+
<< indent(8) << "(MI.getOperand(" << OpNo
743+
<< ").getReg() == MI.getOperand("
744+
<< SourceOperandMap[OpNo].OpInfo.TiedOpIdx
745+
<< ").getReg()) &&\n";
746+
else
747+
PrintFatalError("Unexpected tied operand types!");
748+
}
749+
739750
// We don't need to do anything for source instruction operand checks.
740751
break;
741752
case OpData::Imm:
@@ -773,7 +784,8 @@ void CompressInstEmitter::emitCompressInstEmitter(raw_ostream &OS,
773784

774785
switch (DestOperandMap[OpNo].Kind) {
775786
case OpData::Operand: {
776-
unsigned OpIdx = DestOperandMap[OpNo].OpNo;
787+
unsigned OpIdx = DestOperandMap[OpNo].OpInfo.Idx;
788+
DestRec = DestOperandMap[OpNo].OpInfo.DagRec;
777789
// Check that the operand in the Source instruction fits
778790
// the type for the Dest instruction.
779791
if (DestRec->isSubClassOf("RegisterClass") ||
@@ -783,7 +795,7 @@ void CompressInstEmitter::emitCompressInstEmitter(raw_ostream &OS,
783795
: DestRec->getValueAsDef("RegClass");
784796
// This is a register operand. Check the register class.
785797
// Don't check register class if this is a tied operand, it was done
786-
// for the operand its tied to.
798+
// for the operand it's tied to.
787799
if (DestOperand.getTiedRegister() == -1) {
788800
CondStream.indent(8) << "MI.getOperand(" << OpIdx << ").isReg()";
789801
if (EType == EmitterType::CheckCompress)

0 commit comments

Comments
 (0)