Skip to content

Commit 33f6b10

Browse files
authored
[TableGen][DecoderEmitter] Resolve a FIXME in emitDecoder (#154649)
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 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. See #154477 for the origins of the FIXME.
1 parent d4b9aca commit 33f6b10

File tree

2 files changed

+3
-33
lines changed

2 files changed

+3
-33
lines changed

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

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

88-
static DecodeStatus DecodeXR32RegisterClass(MCInst &Inst, APInt RegNo,
89-
uint64_t Address,
90-
const void *Decoder) {
91-
return DecodeRegisterClass(Inst, RegNo.getZExtValue(), Address, Decoder);
92-
}
93-
9488
static DecodeStatus DecodeXR16RegisterClass(MCInst &Inst, uint64_t RegNo,
9589
uint64_t Address,
9690
const void *Decoder) {
@@ -113,18 +107,6 @@ static DecodeStatus DecodeFPCSCRegisterClass(MCInst &Inst, uint64_t RegNo,
113107
}
114108
#define DecodeFPICRegisterClass DecodeFPCSCRegisterClass
115109

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

llvm/utils/TableGen/DecoderEmitter.cpp

Lines changed: 3 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1264,21 +1264,9 @@ void FilterChooser::emitDecoder(raw_ostream &OS, indent Indent,
12641264
return;
12651265
}
12661266

1267-
for (const OperandInfo &Op : Encoding.getOperands()) {
1268-
// FIXME: This is broken. If there is an operand that doesn't contribute
1269-
// to the encoding, we generate the same code as if the decoder method
1270-
// was specified on the encoding. And then we stop, ignoring the rest
1271-
// of the operands. M68k disassembler experiences this.
1272-
if (Op.numFields() == 0 && !Op.Decoder.empty()) {
1273-
OS << Indent << "if (!Check(S, " << Op.Decoder
1274-
<< "(MI, insn, Address, Decoder))) { "
1275-
<< (Op.HasCompleteDecoder ? "" : "DecodeComplete = false; ")
1276-
<< "return MCDisassembler::Fail; }\n";
1277-
break;
1278-
}
1279-
1280-
emitBinaryParser(OS, Indent, Op);
1281-
}
1267+
for (const OperandInfo &Op : Encoding.getOperands())
1268+
if (Op.numFields())
1269+
emitBinaryParser(OS, Indent, Op);
12821270
}
12831271

12841272
unsigned FilterChooser::getDecoderIndex(DecoderSet &Decoders,

0 commit comments

Comments
 (0)