The InstrInfoEmitter outputs an enum called "OperandType" which gives numerical IDs to each operand type. This patch makes use of this enum to define a function called "getOperandType", which allows looking up the type of an operand given its opcode and operand index.
Details
Diff Detail
- Repository
- rL LLVM
Event Timeline
- Fix some syntax mistakes.
- Map only records that derive from Operand and are not anonymous. This exactly matches the condition used to generate GET_INSTRINFO_OPERAND_TYPES_ENUM.
some syntax nits:
OpcodeOperandTypes [] -> OpcodeOperandTypes[] (removed unnecessary space)
OS << "-1"; -> OS << -1; (don't use string where not necessary
Can the enum be used without this? I'm trying to understand why the enum was there to begin with. Only one in tree target, AVR, defines GET_INSTRINFO_OPERAND_TYPES_ENUM. Its not built by default so I could't really check, but looking through the file that included it, I couldn't prove it was being used.
Indeed, I had the same question myself...
I couldn't find a particular way to use the enum otherwise myself, but I'm not an expert of the relevant code. MCOperandInfo does have a member that is somewhat misleadingly named OperandType, but that one seems to refer to llvm::MCOI::OperandType rather than this enum.
LGTM
Enabling AVR and deleting the generated enum doesn't break anything so I don't think it's been used in-tree. I'm aware of two out-of-tree users (one of them being Nicolas :-)) and presumably AVR has one too as I'd be surprised if they added it without something that needed it given that nobody else has it.
One potential in-tree use I see for OpTypes is using it in TargetInstrInfo::verifyInstruction() to verify operand constraints without having to test the actual instruction. I suppose GISel Combiners could potentially use it to combine instructions while still honouring things like addressing-mode constraints.
utils/TableGen/InstrInfoEmitter.cpp | ||
---|---|---|
356–357 ↗ | (On Diff #205433) | Given that you depend on GET_INSTRINFO_OPERAND_TYPES_ENUM, it may be worth moving this before GET_INSTRINFO_OPERAND_TYPES_ENUM and #define-ing it in this block. It doesn't matter too much though as it's easy enough to have the including source define them both |
372 ↗ | (On Diff #205433) | It would be nice if we could still see the multi-operand operand before expansion but we don't have a need for it so it's fine to leave it out for now |
397 ↗ | (On Diff #205433) | Does it make sense to factor out instructions with the same list to compress the table? I expect the number of rows would drop quite a bit as ISA's generally have a lot of commonality between and/or/xor/etc. and similar. |
404 ↗ | (On Diff #205433) | Whitespace nit after the // |