Eliminates the need for working around optional and token operands being
mistakenly parsed as expressions.
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
For operands that we know should never be considered valid expressions (such as id:... ones maybe?), this can be improved further by parsing them regardless of the instruction they are used in and their position. Also, for register operands we could call parseReg() directly from parseCustomOperand() instead of falling back to that after considering other possibilities in parseOperand().
llvm/utils/TableGen/AsmMatcherEmitter.cpp | ||
---|---|---|
1463–1470 | Apart from allowing calling custom parsers for all operands, this also fixes marking possible positions of operands that come after optional operands. |
llvm/lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp | ||
---|---|---|
379 | Why does only tfe need this? |
llvm/lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp | ||
---|---|---|
379 | An earlier version of the patch used to parse tfe operands into both ImmTyTFEs and tokens, later changed to use ImmTyTFE in all cases for simplicity. I've removed this check and changed the rest of the code to employ parseNamedBit() for parsing hardcoded tfes. Same is done for gds, another operand we use in both the hardcoded and AsmOperand forms. There's a similar situation with off, parsed into tokens while also having parseVReg32OrOff() adding them as ImmTyOffs. I'd prefer to address this separately along with other things like ImmTyIdxen being effectively unused, etc. |
llvm/lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp | ||
---|---|---|
379 | Thanks for the explanation. |
LGTM. Nice improvements!
llvm/lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp | ||
---|---|---|
6045 | I think it is not clear enough that the purpose of this break it to distinguish first and subsequent iterations. | |
6084 | Is it possible to avoid this search on each iteration? Ideally, the code should accumulate cpol values in the loop, but store the result after exiting the loop. Also, I'm not sure if it is possible to have 2 cpol operands separated by other operands. Maybe the search of a preceding cpol operand is not needed. | |
9119 | I think this code is less readable and maintainable than the AMDGPUOptionalOperandTable. |
llvm/lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp | ||
---|---|---|
1821 | Looks like this struct is unused now? |
llvm/lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp | ||
---|---|---|
1821 | Removed in rG536b8c537787b470f6107361b0aab4ff54a1b1ba. |
Why does only tfe need this?