Skip to content

Commit fdc2823

Browse files
committed
[TableGen][DecoderEmitter] Resolve a FIXME in emitDecoder
As the FIXME says, we might generate the wrong code to decode an instruction if it had an operand with no encoding bits. An example is M68k's `MOV16ds` that is defined as follows: ``` dag OutOperandList = (outs MxDRD16:$dst); dag InOperandList = (ins SRC:$src); list<Register> Uses = [SR]; string AsmString = "move.w\t$src, $dst" dag Inst = (descend { 0, 1, 0, 0, 0, 0, 0, 0, 1, 1 }, (descend { 0, 0, 0 }, (operand "$dst", 3))); ``` The `$src` operand is not encoded, but what we see in the decoder is: ```C++ tmp = fieldFromInstruction(insn, 0, 3); if (!Check(S, DecodeDR16RegisterClass(MI, tmp, Address, Decoder))) { return MCDisassembler::Fail; } if (!Check(S, DecodeSRCRegisterClass(MI, insn, Address, Decoder))) { return MCDisassembler::Fail; } return S; ``` This calls DecodeSRCRegisterClass passing it `insn` instead of the value of a field that doesn't exist. DecodeSRCRegisterClass has an unconditional llvm_unreachable inside it. New decoder code looks like: ```C++ tmp = fieldFromInstruction(insn, 0, 3); if (!Check(S, DecodeDR16RegisterClass(MI, tmp, Address, Decoder))) { return MCDisassembler::Fail; } return S; ``` We're still not disassembling this instruction right, but at least we no longer have to provide a weird operand decoder method that accepts instruction bits instead of operand bits.
1 parent 46343ca commit fdc2823

File tree

2 files changed

+3
-34
lines changed

2 files changed

+3
-34
lines changed

llvm/lib/Target/M68k/Disassembler/M68kDisassembler.cpp

Lines changed: 0 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -83,12 +83,6 @@ static DecodeStatus DecodeXR32RegisterClass(MCInst &Inst, uint64_t RegNo,
8383
return DecodeRegisterClass(Inst, RegNo, Address, Decoder);
8484
}
8585

86-
static DecodeStatus DecodeXR32RegisterClass(MCInst &Inst, APInt RegNo,
87-
uint64_t Address,
88-
const void *Decoder) {
89-
return DecodeRegisterClass(Inst, RegNo.getZExtValue(), Address, Decoder);
90-
}
91-
9286
static DecodeStatus DecodeXR16RegisterClass(MCInst &Inst, uint64_t RegNo,
9387
uint64_t Address,
9488
const void *Decoder) {
@@ -111,18 +105,6 @@ static DecodeStatus DecodeFPCSCRegisterClass(MCInst &Inst, uint64_t RegNo,
111105
}
112106
#define DecodeFPICRegisterClass DecodeFPCSCRegisterClass
113107

114-
static DecodeStatus DecodeCCRCRegisterClass(MCInst &Inst, APInt &Insn,
115-
uint64_t Address,
116-
const void *Decoder) {
117-
llvm_unreachable("unimplemented");
118-
}
119-
120-
static DecodeStatus DecodeSRCRegisterClass(MCInst &Inst, APInt &Insn,
121-
uint64_t Address,
122-
const void *Decoder) {
123-
llvm_unreachable("unimplemented");
124-
}
125-
126108
static DecodeStatus DecodeImm32(MCInst &Inst, uint64_t Imm, uint64_t Address,
127109
const void *Decoder) {
128110
Inst.addOperand(MCOperand::createImm(M68k::swapWord<uint32_t>(Imm)));

llvm/utils/TableGen/DecoderEmitter.cpp

Lines changed: 3 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1282,22 +1282,9 @@ bool FilterChooser::emitDecoder(raw_ostream &OS, indent Indent,
12821282
}
12831283

12841284
bool HasCompleteDecoder = true;
1285-
for (const OperandInfo &Op : Encoding.getOperands()) {
1286-
// FIXME: This is broken. If there is an operand that doesn't contribute
1287-
// to the encoding, we generate the same code as if the decoder method
1288-
// was specified on the encoding. And then we stop, ignoring the rest
1289-
// of the operands. M68k disassembler experiences this.
1290-
if (Op.numFields() == 0 && !Op.Decoder.empty()) {
1291-
HasCompleteDecoder = Op.HasCompleteDecoder;
1292-
OS << Indent << "if (!Check(S, " << Op.Decoder
1293-
<< "(MI, insn, Address, Decoder))) { "
1294-
<< (HasCompleteDecoder ? "" : "DecodeComplete = false; ")
1295-
<< "return MCDisassembler::Fail; }\n";
1296-
break;
1297-
}
1298-
1299-
HasCompleteDecoder &= emitBinaryParser(OS, Indent, Op);
1300-
}
1285+
for (const OperandInfo &Op : Encoding.getOperands())
1286+
if (Op.numFields())
1287+
HasCompleteDecoder &= emitBinaryParser(OS, Indent, Op);
13011288
return HasCompleteDecoder;
13021289
}
13031290

0 commit comments

Comments
 (0)