[PATCH] D62674: [ARM] Add a batch of MVE integer instructions.

[Oliver Stannard (Linaro) via Phabricator via llvm-commits]

ostannard added inline comments.


================
Comment at: llvm/lib/Target/ARM/ARMInstrMVE.td:1567
+  : MVE_VQxDMULH<"vqdmulh", suffix, size, pattern> {
+  let Inst{28} = 0b0;
+}
----------------
Bit 28 could be a parameter of MVE_VQxDMULH, and this could be a multiclass to avoid repetition of the types below. (Common pattern in this file).


================
Comment at: llvm/lib/Target/ARM/Disassembler/ARMDisassembler.cpp:3454
+
+  // Might be more than vmov immediate.
+
----------------
What does this mean?


================
Comment at: llvm/test/MC/ARM/mve-integer.s:8
+# CHECK-NOFP: vmov.i32 q0, #0x1bff  @ encoding: [0x81,0xef,0x5b,0x0c]
+vmov.i32 q0, #0x1bff
+
----------------
We should also test that invalid immediates are rejected.


================
Comment at: llvm/test/MC/Disassembler/ARM/mve-integer.txt:5
+
+# CHECK: vmvn.i32 q0, #0x35 @ encoding: [0x83,0xef,0x75,0x00]
+[0x83,0xef,0x75,0x00]
----------------
Should these instructions have CHECK-NOMVE lines?


================
Comment at: llvm/test/MC/Disassembler/ARM/mve-integer.txt:32
+
+# ERROR: [[@LINE+1]]:2: warning: invalid instruction encoding
+[0x83,0xef,0x75,0x0f]
----------------
These ERROR check lines aren't used by and RUN lines.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D62674/new/

https://reviews.llvm.org/D62674