Open danieltowner opened 1 year ago
To be fully general replace basic_simd<Idx, Abi>
with basic_simd<Idx, IdxAbi>
, adding a constraint that the sizes match. (I know your implementation doesn't need it...)
I agree that the ctor would be useful but cryptic. More alternatives:
No ctor but enable & document the following pattern:
const std::basic_simd x = std::simd<float>().gather_from(ptr, idx);
I.e. make gather_from
return *this
. If we do this we should consider adjusting copy_from
accordingly.
Named constructor:
const std::basic_simd x = std::simd<float>::gather_construct(ptr, idx);
Just noticed: If there's going to be gather_from
(and copy_from
) member functions that return *this
then we need to overload on lvalue vs. rvalue:
simd gather_from(...) &&;
simd& gather_from(...) &;
The rvalue overload could even be const &&
: we don't need to modify a temporary since its lifetime basically ends with the end of the gather_from
function.
I don't particularly like having gather_from
have to be so careful with its return types - to me that feels like we're trying to patch up problems that arise from going about this in the wrong way.
I like the named constructor better but would we then have to do the same with some of the other constructors? - That is, why would gather_from
be special? Would you also want copy_from_construct
and generator_construct
too (and maybe more)? To me, having a constructor which accepts a pointer seems okay as that seems a fairly obvious thing to want to do to generate a SIMD value, but having special constructors to deal with the other scenarios seems less reasonable and introduces inconsistencies if we don't apply the same idea to the other available constructors.
Should we go back to the original plan of having them as free functions? After all, we aren't proposing that permute
, compress
, expand
and others should also be members or constructors, so why is gather_from
different? We can still let it have a mask and flags if we wanted, but it wouldn't do any embedded convert and the conversion would have to be done separately if the user wanted it.
Actually, your question whether permute
, compress
, expand
, ... should be members ties in with https://github.com/mattkretz/std-simd-feedback/issues/80. Did you consider members? What's the "generic code story"? I.e. is there one?
x = x.compress(x > 0).permute([](int i) { return i ^ 1; });
That's easier to read than
x = simd_permute(simd_compress(x, x > 0), [](int i) { return i ^ 1; });
I don't think the argument that rvalue and lvalue simd need to consider lifetime carries much weight here. This is nothing the user would need to know/understand. It just works correctly instead of introducing silent object lifetime bugs. However, we might want to have different names for member functions modifying the object (lvalue) and those returning a new object. Analogy: x += 1
modifies an lvalue and doesn't accept an rvalue on the left, x + 1
returns a new object and works with lvalue and rvalue on the left.
At Varna a poll agreed that
gather
andscatter
should be made member functions calledgather_from
andscatter_to
, to match theircopy_from
andcopy_to
counterparts. The original free functiongather
looked like this:This takes a set of indexes and an iterator, and generates a simd of the same size as there are indexes, with the ith element set to
in[indexes[i]]
. Following the direction of the Varna poll this would change to:This new version gains the ability to do conversions as part of the gather (controlled by the flags and the type of
Iter
in comparison to thevalue_type
of the object invoking the gather. This version also enforces that the number of indexes matches the simd into which the values are being gathered. Agreed?In addition, it was agreed that
gather_from
should take a mask parameter as well, to matchcopy_from
:Note that the mask is the object's own
mask_type
since it is the values of the object's own elements which are being masked. Flags have been left as the last parameter where they can default. The first two parameters are left as the iterator and the mask so that they match the order ofcopy_from
, and the indexes are made to be an extra parameter passed after the basic it/mask information.A disadvantage of a member function
gather_from
is that there needs to be an existing object from which to call the method. This is also true ofcopy_from
and in that case this is solved by providing a constructor with the same arguments. Should we therefore extendbasic_simd::basic_simd
overloads to include the two following constructors as well:The reason for calling the free function
gather
, orgather_from
was to make it explicit in the source code what operation was happening, but I think that the most common use case would be to gather directly into a new basic_simd object using a constructor rather than overwriting something already existing, in which case thegather_from
name rarely gets used, and the purpose of the code becomes more obscure. It also mean that the set of overloads on the basic_simd constructor is getting ever bigger.I like the new name, I like the addition of the mask parameter, and I like allowing flags for future expansion (e.g., streaming). But I am now wondering whether it is better to have it as a free function after all. If we consider
permute
:This returns a new simd of the same elements as value, with the size of indexes. If we took that as a pattern, then
has a similar pattern, generating the same elements as the iter with the size of indexes. Perhaps it is better to ignore conversions for gather in the same way as we ignored conversions for permute, and require the user to do an extra conversions if they need to?
Or perhaps we keep gather as a member of the indexes instead:
I think this is less intuitive and it doesn't fit alongside permute, so I don't really like it.
Some of the same thinking applies to
scatter_to
as well, but I think that would be easy to make a member function. Doing so would make it inconsistent withgather_from
though, so I think we need to think about gather first and then how scatter would work in the same way. Having mask and flags in scatter is much easier.Thoughts @mattkretz @rarutyun