[PATCH] D39089: [AArch64][SVE] Asm: Add SVE (Z) Register definitions and parsing support

[Sander de Smalen via Phabricator via llvm-commits]

sdesmalen added inline comments.


================
Comment at: lib/Target/AArch64/AsmParser/AArch64AsmParser.cpp:844
+    return (Kind == k_SVERegister ||
+            (Kind == k_Register && Reg.Kind == RegKind::SVEDataVector)) &&
+           AArch64MCRegisterClasses[Class].contains(getReg());
----------------
rengolin wrote:
> rengolin wrote:
> > To @javed.absar comment in D39088, this may be simplified by having all done at the RegKind level, instead of having multiple types of registers on top of multiple types of SVE vectors.
> I'm confused now. Why do we need both ways of identifying an SVE register?
> 
> I can't see anything in this patch that needs `k_SVEDataRegister`. All uses of it could very well be together with `k_Register` and have an additional check for `RegKind::SVEDataVector`.
Okay, I understand the confusion, and its mostly caused by this patch not containing the complete context of how future patches extend SVEDataRegister. I'll try and provide some more context in my future patches, but let me describe this particular instance here:

k_SVEDataRegister will be extended with more details such as 'ElementWidth' and details about sign/shift extension. In contrast, k_Register and Reg.Kind == RegKind::SVEDataVector describe a vector register as a 'bare' register. At this stage, it is functionally equivalent to using k_Register, so this is more a non-functional change for now to prepare for future patches.


https://reviews.llvm.org/D39089