[PATCH] D45427: [AArch64][AsmParser] Unify code for parsing Neon/SVE vectors.

[Sander de Smalen via Phabricator via llvm-commits]

sdesmalen added inline comments.


================
Comment at: lib/Target/AArch64/AsmParser/AArch64AsmParser.cpp:1866
+    Res = StringSwitch<std::pair<int, int> >(Suffix.lower())
+              .Case("", { 0, 0 })
+              .Case(".1d", { 1, 64 })
----------------
fhahn wrote:
> Why are we matching "" for neon here? Looks like we did not match that before (on a first look)?
That is because this function can also be called when the Suffix is an empty string, where previously 'isValidVectorKind()' was only called when Kind was non-empty. See for instance 'tryParseNeonVectorRegister()' which unconditionally calls 'parseVectorKind()' in order to pass ElementWidth to the CreateVectorReg() call.


================
Comment at: lib/Target/AArch64/AsmParser/AArch64AsmParser.cpp:2877
+      const auto &Res = parseVectorKind(Kind, RegKind::NeonVector);
+      assert(Res && "Expected a valid vector kind");
+      return true;
----------------
fhahn wrote:
> This will trigger an unused variable warning when assertions are enabled I think. Maybe just move parseVectorKind in the assert?
If I put the 'parseVectorKind' itself in an assert, it might not get executed (which is needed for 'Kind' to be set, which is passed by reference). I'll rewrite it into a condition with the  'else' being a llvm_unreachable.


https://reviews.llvm.org/D45427