This change allows to specify "DefaultMethod" for optional operand (IsOptional = 1) in AsmOperandClass that return default value for operand. This is used in convertToMCInst to set default values in MCInst.
Previously if you wanted to set default value for operand you had to create custom converter method. With this change it is possible to use standard converters even when optional operands presented.
Details
Diff Detail
Event Timeline
I haven't looked closely yet, but have you considered doing this entirely in tablegen, either via default operand lists (like ARM's OptionalDefOperand) or even some default value string?
lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp | ||
---|---|---|
396–398 | These look independent; feel free to commit right away (or submit another patch). |
I actualy thought about this but decided that DefaultMethod is better (at least for now) because usage of OptionalDefOperand or default string value would require big changes in MCTargetAsmParser’s interface and all back-ends. MCTargetAsmParser need some way to create arguments (MCParsedAsmOperand) with default values and insert them in instructions. Currently MCTargetAsmParser has no interface to create operands and it’s hard to create it because every target’s MCParsedAsmOperand implementation differs a lot. With DefaultMethod responsibility to create default arguments lies on back-end developers and no interface changes in MCTargetAsmParser required.
Another solution can be that MCTargetAsmParser would directly create MCOperands instead of MCParsedAsmOperand. But in this case it is there is no way to use RenderMethod for this operand.
I'm no fan of this, but I think you're right, there's not much better we can do right now.
What do you think of only generating the extra code only if there's at least one optional operand class? That would avoid needing a mask on other targets.
include/llvm/Target/Target.td | ||
---|---|---|
623 | return default -> returns the default | |
623–624 | "This method is only used if.." | |
625 | IsOptional == 1, no? | |
628–630 | Why pass anything? You don't use the Opcode, and the Operands are only used for the location:
| |
utils/TableGen/AsmMatcherEmitter.cpp | ||
206 | which method? |
Fixes and new code to check if target has optional operands.
include/llvm/Target/Target.td | ||
---|---|---|
628–630 | I added Opcode and Operands to arguments list because this arguments seems to be useful in defaultMethod. It is not hard to imagine situation when default value for operand differs depending on instruction opcode or other operands presented in instruction. But if you think that they are unnecessary I can remove them. There is always first operand in OperandVector - instruction mnemonic. |
include/llvm/Target/Target.td | ||
---|---|---|
628–630 |
Eh, if you have a patch in mind that would use them, OK, but otherwise, what do you think of leaving them out, and adding them when they turn out to be necessary?
You're right, forgot about that! |
return default -> returns the default