c-rhodes added inline comments.
================
Comment at: clang/lib/Sema/SemaCast.cpp:2217-2218
- // Allow reinterpret_casts between vectors of the same size and
- // between vectors and integers of the same size.
bool destIsVector = DestType->isVectorType();
----------------
nit: not sure we need to change this
================
Comment at: clang/lib/Sema/SemaCast.cpp:2763-2767
+ if (SrcType->isVectorType() || DestType->isVectorType())
+ if (Self.isValidSveBitcast(SrcType, DestType)) {
+ Kind = CK_BitCast;
+ return;
+ }
----------------
I think braces are recommended on the outer if as well, see:
https://llvm.org/docs/CodingStandards.html#don-t-use-braces-on-simple-single-statement-bodies-of-if-else-loop-statements
Although I suppose it could also be written as:
```
if ((SrcType->isVectorType() || DestType->isVectorType()) &&
Self.isValidSveBitcast(SrcType, DestType)) {
Kind = CK_BitCast;
return;
}```
================
Comment at: clang/lib/Sema/SemaCast.cpp:2222-2227
+ // Allow bitcasting if either the source or destination is a scalable
+ // vector.
+ if (SrcType->isSizelessBuiltinType() || DestType->isSizelessBuiltinType())
{
+ Kind = CK_BitCast;
+ return TC_Success;
+ }
----------------
joechrisellis wrote:
> c-rhodes wrote:
> > This is a bit ambiguous, it'll allow bitcasting between things like a fixed
> > predicate and any sizeless type which I don't think makes sense. I think
> > it'll also allow bitcasting between any scalable and GNU vectors regardless
> > of vector length, e.g.:
> >
> > ```
> > typedef int32_t int32x4_t __attribute__((vector_size(16)));
> >
> > void foo() {
> > svint32_t s32;
> > int32x4_t g32;
> > g32 = (int32x4_t)s32;
> > s32 = (svint32_t)g32;
> > }
> > ```
> >
> > the above should only work when `-msve-vector-bits=128` but will currently
> > be allowed for any N.
> Ah, you're dead right. I think the next diff should fix this, but if there
> are any more inconsistencies/ambiguities I'll fix them too. :)
> Ah, you're dead right. I think the next diff should fix this, but if there
> are any more inconsistencies/ambiguities I'll fix them too. :)
I suppose it's outside of the scope of this ticket, but I think we'll still
need to address support for explicit conversions between scalable vectors and
GNU vectors like in the example I gave.
================
Comment at: clang/lib/Sema/SemaExpr.cpp:7209-7210
+ auto ValidScalableConversion = [](QualType FirstType, QualType SecondType) {
+ if (!FirstType->getAs<BuiltinType>())
+ return false;
+
----------------
Missing a check for `isSizelessBuiltinType`
================
Comment at: clang/test/SemaCXX/aarch64-sve-explicit-casts-fixed-size.cpp:22-68
+#define TESTCASE(from, to) \
+ void from##_to_##to() {\
+ from a; \
+ to b; \
+ \
+ b = (to) a; \
+ }
----------------
nit: could simplify this a little further by wrapping the macro, e.g. (moving
existing `TESTCASE` to `CAST`):
```#define TESTCASE(ty1, ty2) \
CAST(ty1, ty2) \
CAST(ty2, ty1)```
================
Comment at: clang/test/SemaCXX/aarch64-sve-explicit-casts-fixed-size.cpp:24-25
+ void from##_to_##to() {\
+ from a; \
+ to b; \
+ \
----------------
nit: can just pass these as args `void from##_to_##to(from a, to b) {\`
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D91262/new/
https://reviews.llvm.org/D91262
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
