[PATCH] D138492: [AMDGPU][AsmParser] Refine parsing instruction operands.
Dmitry Preobrazhensky via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Wed Nov 23 06:51:29 PST 2022
dp accepted this revision.
dp added a comment.
This revision is now accepted and ready to land.
LGTM. Nice improvements!
================
Comment at: llvm/lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp:6036
+ else
+ break;
+ } else if (trySkipId("glc"))
----------------
I think it is not clear enough that the purpose of this `break` it to distinguish first and subsequent iterations.
================
Comment at: llvm/lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp:6075
+ AMDGPUOperand *CPolOp = nullptr;
+ for (unsigned I = 1; I != Operands.size(); ++I) {
+ AMDGPUOperand &Op = ((AMDGPUOperand &)*Operands[I]);
----------------
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.
================
Comment at: llvm/lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp:9110
+OperandMatchResultTy
+AMDGPUAsmParser::parseCustomOperand(OperandVector &Operands, unsigned MCK) {
----------------
I think this code is less readable and maintainable than the `AMDGPUOptionalOperandTable`.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D138492/new/
https://reviews.llvm.org/D138492
More information about the llvm-commits
mailing list