bsmith added inline comments.

================
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)
----------------
bsmith wrote:
> paulwalker-arm wrote:
> > 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`?
> I think I would rather have a separate check for min == max, and output a 
> diagnostic stating that an exact -msve-vector-bits value must be used, does 
> that sound sensible?
> 
> This could then perhaps be combined with the previous comment to produce 
> sensible output, i.e.:
> 
> !vscale_min -> not supported
> vscale_min != vscale_max -> must be fixed value
Actually, now that I re-read the error message, and the fact that is directly 
refers to the valid values, I think instead there should just be an initial 
check of `min == 0 || min != max`.


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