simoll planned changes to this revision.
simoll marked 6 inline comments as done.
simoll added inline comments.
================
Comment at: clang/docs/LanguageExtensions.rst:492
+
+The memory representation of a boolean vector is the smallest fitting
+power-of-two integer. The alignment is the alignment of that integer type.
This
----------------
rsandifo-arm wrote:
> It might be worth clarifying this. With the alignment referring specifically
> to “integer type”, I wasn't sure what something like:
>
> bool __attribute__((vector_size(256)))
>
> would mean on targets that don't provide 256-byte integer types. Is the type
> still 256-byte aligned?
Not exactly: It is the alignment of the corresponding integer type.
For example the following C code:
typedef bool bool256 __attribute__((vector_size(256)));
bool256 P;
gives you (x86_64 host, no machine flags specified):
%P = alloca i2048, align 32
================
Comment at: clang/include/clang/AST/Type.h:3248
+ bool isVectorSizeBoolean() const {
+ return (getVectorKind() == VectorKind::GenericVector) &&
----------------
rsandifo-arm wrote:
> Maybe isGenericBooleanVector(), so that the terminology is consistent? Just
> a suggestion though, not sure if it's an improvement.
I gave it this specific name so where ever it is used, it documents that
whatever depends on it is specific to vector_size bool. This should be cleaned
up when a better boolean vector type is introduced.
================
Comment at: clang/lib/Sema/SemaExpr.cpp:9779
+ return Ty->isBooleanType() ||
+ (Ty->isVectorType() &&
+ Ty->getAs<VectorType>()->getElementType()->isBooleanType());
----------------
rsandifo-arm wrote:
> Is this deliberately wider than isVectorSizeBoolean(), or does it amount to
> the same thing?
That's an artifact. Will narrow this down to `isVectorSizeBoolean()`.
================
Comment at: clang/lib/Sema/SemaExpr.cpp:11929
VectorType::GenericVector);
+ else if (TypeSize == Context.getTypeSize(Context.BoolTy))
+ return Context.getVectorType(Context.BoolTy, VTy->getNumElements(),
----------------
rsandifo-arm wrote:
> In practice, won't this either be a no-op (e.g. for 4-byte-per-bool targets)
> or always trump the fallback CharTy case? I wasn't sure why the case was
> needed.
I guess so. Will remove this case.
================
Comment at: clang/lib/Sema/SemaExpr.cpp:11956
+ /*AllowBoolConversions*/ getLangOpts().ZVector,
+ /*AllowBooleanOperation*/ false);
if (vType.isNull())
----------------
rsandifo-arm wrote:
> Seems like it might be useful to support comparisons too (with false < true,
> as for scalars). E.g. I guess x == y would otherwise need to be written ~(x
> ^ y), which seems less readable. Supporting more operators would also help
> in templated contexts.
Agreed.
================
Comment at: clang/lib/Sema/SemaExprCXX.cpp:6271
+ /*AllowBoolConversions*/ false,
+ /*AllowBoolOperator*/ false);
----------------
rsandifo-arm wrote:
> Any reason not to support this for booleans? ?: seems like a natural
> operation for all vector element types.
Sure, will allow it.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D81083/new/
https://reviews.llvm.org/D81083
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
