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