This patch allows the same implicit conversions for vector-scalar
operations in SVE that are allowed for NEON.
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
Looking pretty good. A couple of test cases to consider:
#include <arm_neon.h> #include <arm_sve.h> svint8_t svi8(svint8_t a) { return a + 256; } int8x16_t nei8(int8x16_t a) { return a + 256; } svint8_t svi8_128(svint8_t a) { return a + 128; } int8x16_t nei8_128(int8x16_t a) { return a + 128; }
inp.c:4:13: error: invalid operands to binary expression ('svint8_t' (aka '__SVInt8_t') and 'int') return a + 256; ~ ^ ~~~ inp.c:7:11: error: cannot convert between scalar type 'int' and vector type 'int8x16_t' (vector of 16 'int8_t' values) as implicit conversion would cause truncation return a + 256; ^ inp.c:13:13: warning: implicit conversion from 'int' to 'int8x16_t' (vector of 16 'int8_t' values) changes value from 128 to -128 [-Wconstant-conversion] return a + 128; ~ ^~~
Note that for NEON we get a warning about the implicit conversion, and a clear error message about truncation, but for SVE there is no warning and simply an 'invalid operands to binary expression', which could be confusing since the implicit conversions exist for the same operand type with differing value.
clang/lib/Sema/SemaExpr.cpp | ||
---|---|---|
10588 | !Ptr && [Deref Ptr] looks suspicious. Also, if I insert an abort in this path and run check-clang I don't see any failing tests? It might help to name LHSBuiltinTy && LHSBuiltinTy->isVLSTBuiltinType() IsLHSVector (likewise for RHS) or similar. |
I think this needs more tests to ensure every diagnostic is being triggered correctly so I will be adding those in another patch, but I wanted to put up the fix for the original issue as soon as possible!
can the diagnostics improvements be broke out into separate patches? I think it would make this easier to review
I've split the diagnostic changes from this out as D126377 and will repost this patch soon based on that one. Thanks!
!Ptr && [Deref Ptr] looks suspicious. Also, if I insert an abort in this path and run check-clang I don't see any failing tests?
It might help to name LHSBuiltinTy && LHSBuiltinTy->isVLSTBuiltinType() IsLHSVector (likewise for RHS) or similar.