I noticed that the TableGen code emitter generator assumes that the instruction fields are declared in the instruction format in the same order that operands are defined. This seems like a bad dependence to me, and that TableGen should match the name of field declared in the instruction with the name of the operand in order to determine which operand of the MI to use
.
See CodeEmitterGen.cpp:170 and neighborhood.
Unfortunately there are naming discrepancies between the instruction format fields and the operand names in all existing code emitters that use TableGen, which would require some significant renaming cleanup.
Thoughts on binding operand names to instruction field names for the code emitter?
I noticed that the TableGen code emitter generator assumes that the instruction fields are declared in the instruction format in the same order that operands are defined. This seems like a bad dependence to me, and that TableGen should match the name of field declared in the instruction with the name of the operand in order to determine which operand of the MI to use
.
Yes, instruction OperandList corresponds to MachineInstr operands ordering. Are you saying MachineInstr should look up specific operand by name?
See CodeEmitterGen.cpp:170 and neighborhood.
Unfortunately there are naming discrepancies between the instruction format fields and the operand names in all existing code emitters that use TableGen, which would require some significant renaming cleanup.
Not sure what you mean. Example?
Evan
I noticed that the TableGen code emitter generator assumes that the instruction fields are declared in the instruction format in the same order that operands are defined. This seems like a bad dependence to me, and that TableGen should match the name of field declared in the instruction with the name of the operand in order to determine which operand of the MI to use
.
Yes, instruction OperandList corresponds to MachineInstr operands ordering. Are you saying MachineInstr should look up specific operand by name?
See CodeEmitterGen.cpp:170 and neighborhood.
Unfortunately there are naming discrepancies between the instruction format fields and the operand names in all existing code emitters that use TableGen, which would require some significant renaming cleanup.
Not sure what you mean. Example?
I’m saying that the GenCodeEmitter tablegen back end should look up MI operand number by name, rather than assuming the order of declarations matches the order of the operands. This way tablegen can emit an error if the field name and operand name fail to match rather than silently producing an incorrect GenCodeEmitter.
class MyFormat<bits<32> OpcVal, dag ops, string asmstr, list pattern, InstrItinClass itin> :
MyInst<ops, asmstr, pattern, itin> {
let Inst{31-28} = OpcVal{31-28};
bits<6> Rsrc1; <===== Because destination is required first in operand ordering,
bits<6> Rdest; <===== this ordering of declarations produces an incorrect GenCodeEmitter
bits<16> Imm16;
let Inst{27-22} = Rsrc1;
let Inst{21-16} = Rdest;
let Inst{15-0} = Imm16;
}
def ORI : MyFormat<0, (ops GPRegs:$Rdest, GPRegs:$Rsrc1, i32imm:$Imm16), …
In the generated emitter (wrong):
Rsrc1 ← MI.Operand(0)
Rdest ← MI.Operand(1);
However, there would be quite a bit of renaming required to get the instruction format fields to match up with the MI operand names for all the back ends that currently automatically generate their code emitters.
I noticed that the TableGen code emitter generator assumes that the instruction fields are declared in the instruction format in the same order that operands are defined. This seems like a bad dependence to me, and that TableGen should match the name of field declared in the instruction with the name of the operand in order to determine which operand of the MI to use
.
Yes, instruction OperandList corresponds to MachineInstr operands ordering. Are you saying MachineInstr should look up specific operand by name?
See CodeEmitterGen.cpp:170 and neighborhood.
Unfortunately there are naming discrepancies between the instruction format fields and the operand names in all existing code emitters that use TableGen, which would require some significant renaming cleanup.
Not sure what you mean. Example?
I’m saying that the GenCodeEmitter tablegen back end should look up MI operand number by name, rather than assuming the order of declarations matches the order of the operands. This way tablegen can emit an error if the field name and operand name fail to match rather than silently producing an incorrect GenCodeEmitter.
class MyFormat<bits<32> OpcVal, dag ops, string asmstr, list pattern, InstrItinClass itin> :
MyInst<ops, asmstr, pattern, itin> {
let Inst{31-28} = OpcVal{31-28};
bits<6> Rsrc1; <===== Because destination is required first in operand ordering,
bits<6> Rdest; <===== this ordering of declarations produces an incorrect GenCodeEmitter
bits<16> Imm16;
let Inst{27-22} = Rsrc1;
let Inst{21-16} = Rdest;
let Inst{15-0} = Imm16;
}
def ORI : MyFormat<0, (ops GPRegs:$Rdest, GPRegs:$Rsrc1, i32imm:$Imm16), …
In the generated emitter (wrong):
Rsrc1 ← MI.Operand(0)
Rdest ← MI.Operand(1);
However, there would be quite a bit of renaming required to get the instruction format fields to match up with the MI operand names for all the back ends that currently automatically generate their code emitters.
Right. This is a nice to have that’s unlikely to be tackled anytime soon. Please file a enhancement request.
Evan
FWIW, I completely agree with you.
-Chris