Finish the removal of Simd::IsStandard

Simd::IsStandard has been disabled in !509 (merged) to fix #121 (closed). This issue is a reminder to remove it completely.

!509 (merged) seems to fix the issues in OPM. Thanks.

Turns out, the prioritization of which adaptation layer is getting used did not work as intended for the traits classes anyway (e.g. Simd::Index, ambigous overload, both the standard abstraction and the vc abstraction wanted to handle the Vc types...)

Gonna have to take another look at it.

mentioned in commit 069d2549

mentioned in merge request !510 (merged)

assigned to @joe

Gah. I now remember the other thing IsStandard was useful for. Problem description, mostly so I have it written down somewhere, and can organize my thoughts. (I know I figured this out before at some point, but can't find it...)

The overloads that implement the interface use a priority argument ADLTag<prio> (similar to Dune::PriorityTag<prio>) to be able to prioritize overloads with respect to each other. To explain the problem, I'm going to consider the function lane(l, vec), which provides access to lane l of the SIMD vector vec.

• If vec is a non-const lvalue, lane(l, vec) can be assigned to, either because it returns an lvalue reference directly to the vector entry, or because it returns a proxy object that implements the assignment.
• If vec is anything else, the return value of lane(l, vec) may be a prvalue of the extracted lane, or an rvalue reference into the vector, or a const lvalue reference into the vector.

For the purpose of this discussion, the lane parameter l does not matter, so I'll leave it of.

One implementation of ADLTag could be:

namespace Overloads {
  template<std::size_t prio> struct ADLTag : ADLTag<prio-1> {};
  template<> struct ADLTag<0> {};
}

The interface lane function can then be implemented as

template<class V>
decltype(auto) lane(V &&vec) {
  return lane(Overloads::ADLTag<6>{}, std::forward<V>(vec));
}

The adapter for standard types uses (mostly arbitrarily) priority 2. Since there is no IsStandard anymore, it has no way to identify the types it should apply to, and must apply to all types. The current implementation therefore is

namespace Overloads {
  template<class V>
  decltype(auto) lane(ADLTag<2>, V &&vec) { return std::forward<V>(vec); }
}

I.e. return the "vector" itself, with whatever value category and const-qualification we got it.

Adapters for specific SIMD libraries use priority 5, which is also mostly arbitrary, except that it is higher than the one used for standard types. An implementation for e.g. MySIMD might look something like this:

namespace Overloads {
  template<class T>
  T& lane(ADLTag<5>, MySIMD::Vector<T> &vec) { return vec[0]; }

  template<class T>
  T lane(ADLTag<5>, const MySIMD::Vector<T> &vec) { return vec[0]; }
}

This would nicely implement the lane function, including that its result is assignable if the argument is a non-const lvalue. The const lvalue version can also bind rvalues, and returning a prvalue does the right thing. Since the types are SIMD types and can be passed around in registers anyway, we don't necessarily need to avoid copies for performance reasons.

We still need to handle incoming references correctly though -- and here we have a problem. The call lane(MySimd::Vector<int>{}) will forward the vector as an rvalue reference. It will look for a function matching lane(Overloads::ADLTag<6> &&, MySIMD::Vector<int> &&). Both the adaptor for standard types and the const MySIMD::vector<T> & function are viable candidates, and it comes down to which one is a better match. For the first argument, the "MySIMD" version is better, because it has higher priority, for the second argument the "standard" version is better because it uses a universal reference and therefore counts as an exact match with no conversion required. And we have an ambiguous overload.

Note that the problem would not go away if I would do away with the priority tag. Yes, there would be no more ambiguity, but it would select the "standard" candidate, which is still wrong. I would just get a compilation error later (or at least I hope I would, the alternative being code that silently does the wrong thing).

1. One way to resolve this is for the implementor of the MySIMD adaptor to explicitly add an overload that takes an rvalue argument:

namespace Overloads {
  template<class T>
  T lane(ADLTag<5>, MySIMD::Vector<T> &&vec) { return vec[0]; }
}

This has the disadvantage that it must be done for every function that needs implementing, and it is code duplication since it is essentially the same as the const lvalue-version. But I tested it and it works.

2. Preferably, I'd like to provide all three relevant versions of const-ref-qualification for the "standard" adaptor. But the rvalue-version of that would still be a better match for a MySIMD rvalue than const MySIMD::Vector<T> & version, so that does not help.

3. In the "standard" adaptor, I could take the vec argument by value (V vec) instead of by reference (copying is cheap here). But I need to handle non-const lvalue arguments specially, since I cannot simply return a prvalue for them. And I cannot simply add a V& vec overload, since overload resolution thinks it is equally good as the V vec overload, resulting in ambiguity.

4. This one may be the winner. Like 3) but give the V& vec overload a higher priority, e.g. make it

namespace Overloads {
  template<class V>
  V lane(ADLTag<2>, V vec) { return vec; }

  template<class V>
  V &lane(ADLTag<3>, V &vec) { return vec; }
}

The const MySIMD::Vector<T> & overload seems to be a better match than the "standard" by-value overload. (coliru, note though that the lvalue ref overload of the standard adaptor also applies to const lvalue ref arguments, though in that case the deduced V includes that const-qualifier).

TODO:

• in 4), what if MySIMD elects to provide a by-value overload instead of a const-lvalue-ref overload? Does it still work out?
• adjust priority description in base.hh to allocate multiple priorites per adaptor. Currently, priorites 1, 3, 4 and 6 are basically there to allow for hackery.
• Ideally, find a way to make this easy to remember (unlikely to happen :/ ).

Hmm.

• My assumption that pass-by-value is "cheap enough" may not be true especially for the non-simd case. Think e.g. AD types.
• A trait that determines whether a type is to be handled by the standard adaptor may not the worst solution after all.
• A similar problem should occur with the default implementation. In principle, apparently it did not yet occur in practice with the current set of interface functions. Or it did occur, but was not noticed, because the default implementation was used and did the right thing (except slower).
• A simple "this type is handled otherwise" trait would not be sufficient for the default implementation, because some functions there take arguments of different types. (For the standard implementation, we can disable it if at least one argument is not a standard type, usually.)

mentioned in commit f2e18c25

mentioned in merge request !514 (merged)

closed via merge request !514 (merged)

mentioned in commit ec9401e6