[PATCH] D19584: AMDGPU/SI: Assembler: Unify parsing/printing of operands.

[Nikolay Haustov via llvm-commits]

nhaustov marked 5 inline comments as done.
nhaustov added a comment.

It's hard to split this change because operand changes depend on parsing changes and vice versa.


================
Comment at: lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp:74-75
@@ -70,4 +73,4 @@
     ImmTyTFE,
-    ImmTyClamp,
-    ImmTyOMod,
+    ImmTyClampSI,
+    ImmTyOModSI,
     ImmTyDppCtrl,
----------------
SamWot wrote:
> Why do we need to rename Clamp and OMod?
Now print and parse function names correspond to name in .td file. It now conflicts with Clamp / printClamp for pre-SI clamp.

================
Comment at: lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp:550
@@ -529,1 +549,3 @@
+                                          int64_t Default = 0, bool AddDefault = false,
+                                          bool (*ConvertResult)(int64_t&) = 0);
   OperandMatchResultTy parseNamedBit(const char *Name, OperandVector &Operands,
----------------
SamWot wrote:
> Consider using std::function instead of function pointer
This matches definition in OptionalOperand struct. It may actually become a virtual method in operand class.

================
Comment at: lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp:1953
@@ -1974,6 +1952,3 @@
 
-  if (Operands.size() > 3) {
-    AMDGPUOperand &Src1Op = ((AMDGPUOperand&)*Operands[3]);
-    if (Src1Op.isRegClass(AMDGPU::SReg_32RegClassID) ||
-        Src1Op.isRegClass(AMDGPU::SReg_64RegClassID))
-      return true;
+AMDGPUAsmParser::OperandMatchResultTy AMDGPUAsmParser::parseOptionalOperand(OperandVector &Operands, const OptionalOperand& Op, bool AddDefault)
+{
----------------
SamWot wrote:
> Support for sdwa should be added here. It might be done later.
Yes, I'll leave it for you.


http://reviews.llvm.org/D19584