[PATCH] D36792: [AArch64] v8.3-a complex number support

[Sjoerd Meijer via Phabricator via llvm-commits]

SjoerdMeijer added inline comments.


================
Comment at: lib/Target/AArch64/AArch64InstrFormats.td:9411
+}
+def ComplexRotationOddOperand : AsmOperandClass {
+  let Name = "ComplexRotationOdd";
----------------
I don't think we need 2 operand classes. They are essentially the same things, just some constants are different. 
An example from the ARM backend is:   ImmAsmOperand<int Low, int High>, where we pass the range of the imm value. I think we can do something similar here. We can then also avoid some duplication in the print and predicate functions, see also comments below.


================
Comment at: lib/Target/AArch64/AsmParser/AArch64AsmParser.cpp:838
 
+  bool isComplexRotation() const {
+    if (!isImm()) return false;
----------------
We can refactor this and the next function (and create only 1), and use "PredicateMethod" in the AsmOperand class.


================
Comment at: lib/Target/AArch64/AsmParser/AArch64AsmParser.cpp:1550
 
+  void addComplexRotationOperands(MCInst &Inst, unsigned N) const {
+    assert(N == 1 && "Invalid number of operands!");
----------------
We can refactor this function and the next one and create one (template/parametric) function to avoid code duplication (see earlier comment about the operand classes).


================
Comment at: lib/Target/AArch64/InstPrinter/AArch64InstPrinter.cpp:1335
+
+void AArch64InstPrinter::printComplexRotationOp(const MCInst *MI, unsigned OpNo,
+                                                const MCSubtargetInfo &STI,
----------------
Same here.


https://reviews.llvm.org/D36792