[DPC++] reduction error using a vector type

[zjin-lcf]

The reproducer is listed below. A comment is added to the line of issue which causes build errors. Thank you.

#include <CL/sycl.hpp>
#include <iostream>
#include <random>

using namespace sycl;
using namespace sycl::ONEAPI;

int main() {
  constexpr size_t N = 16;
  constexpr size_t L = 4;

  queue Q;
  float* data = malloc_shared<float>(N, Q);
  float* data2 = malloc_shared<float>(N, Q);

  float2* res = malloc_shared<float2>(1, Q);
  std::generate(data, data + N, std::mt19937{});
  std::generate(data2, data2 + N, std::mt19937{});

  float2 identity = {0.f, 0.f};
  *res = identity;
  auto red = reduction(res, identity, std::plus<float2>());
  Q.submit([&](handler& h) {
     h.parallel_for(nd_range<1>{N, L}, red, [=](nd_item<1> item, auto& res) {
       int i = item.get_global_id(0);
       res += {data[i], data2[i]};   //  res.combine({data[i], data2[i]});  is successful 
     });
   }).wait();

  std::cout << "sum value = " << res->x() << " " << res->y() << "\n";

  free(res, Q);
  free(data, Q);
  free(data2, Q);
  return 0
}

[Pennycook]

@v-klochkov: As far as I can tell, the issue here is that the shorthand operators are only declared inside of the specialization of reducer that is used for reductions with known identity values.

Should the shorthand operators be duplicated in the base class (or maybe just moved there)?

[v-klochkov]

Should the shorthand operators be duplicated in the base class (or maybe just moved there)?

I agree. This test shows that there is no any need in adding constraints for reduction operators like operator+=.
If += is used in lambda function and user's type does not support it, then it would be reported to user, and that can be fixed immediately for user's type.

[v-klochkov]

@Pennycook , I think some constraints for known type/operation may be useful.
E.g., if the reduction operator is std::plus, then it would be good to have a compilation error if lambda function has "Sum *= value;".
The alternative is to say it is undefined behavior at runtime.

[v-klochkov]

Another alternative, which seems a bit odd, just wanted to mention it.

reducer &operator+=(const T &Partial) {
combine(Partial);
return *this;
}

reducer &operator*=(const T &Partial) {
combine(Partial);
return *this;
}

Here if the reduction operator is std::plus, then using "Sum *= value" by mistake in the lambda function will have the result of "Sum.combine(value)", which is equivalent of "Sum += value;"

[Pennycook]

You're right, there are several options here that I hadn't considered.

I think we should keep the IsReduPlus check (and equivalent checks for other operators) but move the operators to the base class, so that they're available for all T.

That should ensure the shorthands are always available for the built-in functional types, but only when they match. We should throw an error if the user tries to use a mismatched reduction operator, partly to keep code readable and partly to avoid hard-to-find bugs.

[v-klochkov]

Initial version of a fix for this issue: #3193
LIT test: intel/llvm-test-suite#140