[PATCH] D62673: [ARM] Add MVE vector bit-operations (register inputs).

[Simon Tatham via Phabricator via llvm-commits]

simon_tatham marked 2 inline comments as done.
simon_tatham added inline comments.


================
Comment at: llvm/lib/Target/ARM/AsmParser/ARMAsmParser.cpp:1947
 
+  template <int size>
+  bool isMVEVectorIndex() const {
----------------
ostannard wrote:
> Do we need a new function for this, rather than re-using the NEON ones above?
I think the point of this one is that it takes the size as a template parameter, which in turn can be passed through from the Tablegen template parameter on the `MVEVectorIndexOperand` class.

I could rename it to `isVectorIndex` and replace existing uses of `isVectorIndex32` with `isVectorIndex<2>`, and similarly with `64` and `<1>`?


================
Comment at: llvm/lib/Target/ARM/AsmParser/ARMAsmParser.cpp:6679
+    // For vmov instructions, as mentioned earlier, we did not add the vector
+    // predication code, since these may contain operands that require
+    // special parsing.  So now we have to see if they require vector
----------------
ostannard wrote:
> Which operand require special parsing? This comment sounds like it's referring to operands with a custom ParserMethod listed in the tablegen description, but this patch doesn't add any of those.
That comment refers back ("as mentioned earlier") to one further up this function, which mentions that the potential problem is that custom parser methods might refer to fixed offsets in the vector of parsed operands.

An example is the NEON `VMOV instruction that takes an FP immediate. The custom parser method for that operand is `parseFPImm`, which checks `Operands[2]` to find out the width of the FP lane (expecting that operand to be a string such as `.f16`).

I think the point of this business is that we're trying not to perturb parsing of the existing instructions with the same mnemonic, by making sure the operand vector looks the same way it used to during parsing, and only inserting the new MVE predicate after all of that stuff has finished running.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62673