simoll added inline comments.
================
Comment at: clang/docs/LanguageExtensions.rst:488-489
+
+The memory representation of a dense boolean vector is the smallest fitting
+integer. The alignment is the number of bits rounded up to the next
+power-of-two but at most the maximum vector alignment of the target. This
----------------
rsmith wrote:
> Given that we support `ExtInt(n)` integer types for arbitrary `n`, I don't
> think this is clear. Perhaps "is the same as that of the lowest-rank standard
> integer type of at least the specified width" or some less wordy way of
> saying the same thing?
>
> What happens if you ask for more vector elements than the bit width of
> `unsigned long long`?
>
> Is the rule "The size and alignment are both the number of bits rounded up to
> the next power of two, but the alignment is at most the maximum vector
> alignment of the target." ?
> Is the rule "The size and alignment are both the number of bits rounded up to
> the next power of two, but the alignment is at most the maximum vector
> alignment of the target." ?
Yes and i like your description better.
================
Comment at: clang/lib/AST/TypePrinter.cpp:654
+ OS << "__attribute__((__vector_size__(" << NumVectorElems;
+ if (!T->isExtVectorBoolean()) {
+ OS << " * sizeof(";
----------------
rsmith wrote:
> If it's an `ext_vector_type`, we shouldn't be printing it as
> `__vector_size__`. `ExtVectorBoolean` isn't really a special case here; we
> should be distinguishing `ExtVectorType` from regular `VectorType` in general.
Undid this change. There is a separate function for `printExtVectorBefore` that
works fine as is.
================
Comment at: clang/lib/CodeGen/CGExpr.cpp:2084-2085
Dst.isVolatileQualified());
+ auto *IRStoreTy = dyn_cast<llvm::IntegerType>(Vec->getType());
+ if (IRStoreTy) {
+ auto *IRVecTy = llvm::FixedVectorType::get(
----------------
majnemer wrote:
> `if (auto *IRStoreTy = dyn_cast<llvm::IntegerType>(Vec->getType()))`
`IRStoreTy` is used here and below
================
Comment at: clang/lib/CodeGen/CGExprScalar.cpp:2055-2057
+ auto *PlainIntTy = llvm::VectorType::get(Builder.getIntNTy(DstElemBits),
+ VecSrcTy->getElementCount());
+ Src = Builder.CreateSExt(Src, PlainIntTy);
----------------
rsmith wrote:
> I don't think we should represent widening a vector of booleans to a mask
> vector of 0/-1 integers as a `CK_BitCast`. `CK_BitCast` is intended for cases
> where the bit-pattern is reinterpreted, not where it's modified.
>
> Can we add a new cast kind for this instead?
The ext_vector_type does not support casting and we do not need it for the bool
vector type - i've removed this code path.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D88905/new/
https://reviews.llvm.org/D88905
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
