paulwalker-arm added inline comments.

================
Comment at: clang/lib/Sema/SemaType.cpp:7893
   // Attribute is unsupported if '-msve-vector-bits=<bits>' isn't specified.
-  if (!S.getLangOpts().ArmSveVectorBits) {
+  if (!S.getLangOpts().VScaleMin) {
     S.Diag(Attr.getLoc(), diag::err_attribute_arm_feature_sve_bits_unsupported)
----------------
This should be `VScaleMax` as the attribute is only valid when 
`-msve-vector-bits=` is given only a number (i.e. `-msve-vector-bits=128+`  
does not enable the fixed length ACLE extension).

I guess we need tests to cover this scenario.


================
Comment at: clang/lib/Sema/SemaType.cpp:7916
   // The attribute vector size must match -msve-vector-bits.
-  if (VecSize != S.getLangOpts().ArmSveVectorBits) {
+  if (VecSize != S.getLangOpts().VScaleMin * 128) {
     S.Diag(Attr.getLoc(), diag::err_attribute_bad_sve_vector_size)
----------------
I'm thinking there should be a check for `S.getLangOpts().VScaleMin == 
S.getLangOpts().VScaleMax` somewhere, but I guess the current diagnostic 
probably doesn't account for two values, nor should it really.  What about just 
duplicating this block but using `VScaleMax` in place of `VScaleMin`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111790

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to