Added broadcast overload for bool #1089
Conversation
9deba5b to
6606bf8
Compare
6606bf8 to
24cf4ad
Compare
| template <class A> | ||
| struct broadcaster<bool, A> | ||
| { | ||
| using return_type = batch_bool<xsimd::as_unsigned_integer_t<bool>, A>; |
There was a problem hiding this comment.
I'm not quite sure about that. You're creating a batch_bool out of bool, which looks fine at first glance, but a batch_bool neeads to know what kind of type it's operating one, why choosing xsimd::as_unsigned_integer_t<bool> ? The user has no way to specify the actual type...
There was a problem hiding this comment.
This method is called only when you want to operate on batches of boolean values (the old batch<bool> that we removed), therefore the underlying type does not really matter. I chose uint8_t so that we can operate on a maximum number of boolean values in the batch_bool.
If you need a specific underlying type for the batch_bool, then you have to use broadcast_as instead.
There was a problem hiding this comment.
ok. I think we need to document that somewhere, either in the doxygen comment or by moving the specializatin at the API level so that the difference in behavior is clear when passing a bool to xsimd::broadcast. Bonus point if you mention xsimd::broadcast_as there ;-)
There was a problem hiding this comment.
Moving the specialization at the API level is what I did initially (where it actually becomes an overload, since we cannot partially specialize functions). The issue is that the overload as only one template parameter, and with the default value of the first overload, the compiler does not pick the right overload, leading to compilation issues for regular cases (i.e. not bool). I'll add more documentation.
|
🙇 |
Fixes missing APIs due to #973
@serge-sans-paille This is the last missing piece to upgrade xtensor to xsimd 13. If you don't have any objection, I will tag a release after this one gets in.