Repository navigation
Lax conversions - #42
danieltowner wants to merge 1 commit into
Conversation
|
I would suggest we are done with |
gcc used lax vector conversion to provide implicit conversions between vectors of the same size but different types. This has been removed so that there is no requirement for gcc users to provide this flag themselves for their projects. A few minor issues that appeared as a consequence were fixed by inserting extra explicit conversions where needed.
b29d395 to
5f22167
Compare
| /// representation. This constructor allows a mask to be built from the output | ||
| /// of an overloaded operator, a builtin, and so on. | ||
| /// @ingroup simd_constructor | ||
| constexpr basic_mask(const builtin_type& m) noexcept : mask(m) {} |
There was a problem hiding this comment.
Please see this comment and tell me if it makes sense.
The new public basic_mask(const builtin_type&) constructor causes wrong results.
The constructor is implicit, and it skips truncate_to_size, which from_builtin applies. On compact-mask targets (AVX-512 and _XVEC_GENERIC_COMPACT), builtin_type is an unsigned integer (unsigned _BitInt(N) on clang, __uint128_t on GCC). So any int or bool now converts to a mask without a word.
| Expression | main | this branch |
|---|---|---|
m && true |
compile error | compiles; count = 1 (only lane 0 set) |
m == true |
compile error | compiles; count = 1 |
M c = 0xFF; |
compile error | compiles; count = 4 but all_of(c) = 0 (unused upper bits left set) |
The reason: true becomes _BitInt value 1, which then becomes a mask. The 0xFF case also shows that the constructor can break the rule that bits above size are zero. A second problem: M c = 0xFFu (copy-init) and M c(0xFFu) now pick different constructors.
I think that adding this constructor is the whole point of this PR, right? In other words, we are making it available to the users, correct?
If that's the case please try to mark it explicit and have it truncate and see if it fixes the problem.
Also, if the problem exists, please also add the respective tests to be able to catch any regression in the future.
This branch has not been deployed
2 participants
No description provided.