[SYCL] Fix sycl::span type deduction

[cperkinsintel]

The sycl::span does not correctly deduce the type from an array argument. Here we fix this by adjusting the deduction guide to select the correct specialization (that has array support). The tests on llvm-test-suite are being updated as well. ( intel/llvm-test-suite#572 )

Signed-off-by: Chris Perkins [email protected]

[steffenlarsen]

LGTM!

[keryell]

I love comments! Thanks. Send more! :-)
But why does it work at the end and why we have _Sz ending in the type eventually?

[cperkinsintel]

@keryell , I think this previous discussion (the one that led to the comment in the first place) answers your question:
#4970 (comment)

But if it doesn't, perhaps you could rephrase it?

Regardless, it appears I should expand the comment even more.

[cperkinsintel]

I think I see what the question is, and maybe I misspoke when answering @steffenlarsen earlier. The _Sz does not end up changing the extent (which is const) but ends up setting the __size of the span.

I guess a variation on these questions might be, "why is an array of known size end up constructing a span with dynamic_extent"? And while not the author of the code, I believe the answer to that is very simply that the dynamic_extent specialization tracks both data and size, while the other specialization does not. So the array constructor was placed there.

[steffenlarsen]

It sounds to me like there's something wrong with the implementation. Correct me if I am wrong, but this way we won't have the "same" type deduced as for std::span, right? I.e.

int arr[] {1,2,3,4};
sycl::span SYCLSpan {arr}; // will have type sycl::span<int, sycl::dynamic_extent>
std::span STDSpan {arr};   // will have type std::span<int, 4>

[cperkinsintel]

Yes, it looks like there is a problem in the implementation. I'll open up a ticket for it, rather than try to address it here. Being unable to instantiate spans at all from an array without a type specifier is a more immediate problem.

I went and compared GCC span, Clang libc++ span, and our SYCL span, and discovered that not only are we not handling fully specialized spans-from-arrays, but, surprisingly, partially specialized ones had dynamic extent in GCC and Clang stdlib, which I wasn't expecting.

int arr[] { 0, 1, 2, 3, 4, 5, 6, 7, 8 };
    
    SPAN<int,9> fully_specialized {arr};
    std::cout << typeid(fully_specialized).name() << std::endl;
    // GCC stdlib : St4spanIiLm9EE                           => std::span<int, 9ul>
    // clang stdlib : NSt3__14spanIiLm9EEE                   => std::__1::span<int, 9ul>
    // SYCL : won't compile. 
    
    SPAN<int> partially_specialized {arr};
    std::cout << typeid(partially_specialized).name() << std::endl;
    // GCC stdlib : St4spanIiLm18446744073709551615EE        => std::span<int, 18446744073709551615ul>
    // Clang stdlib: NSt3__14spanIiLm18446744073709551615EEE => std::__1::span<int, 18446744073709551615ul>
    // SYCL : N2cl4sycl4spanIiLm18446744073709551615EEE      => cl::sycl::span<int, 18446744073709551615ul>

    SPAN  unspecialized {arr};
    std::cout << typeid(unspecialized).name() << std::endl;
    // GCC stdlib : St4spanIiLm9EE                           => std::span<int, 9ul>
    // clang stdlib : NSt3__14spanIiLm9EEE                   => std::__1::span<int, 9ul>
    // SYCL : N2cl4sycl4spanIiLm18446744073709551615EEE      => cl::sycl::span<int, 18446744073709551615ul>

[keryell]

At least this shows it is good to add some comments and it can exhibit some bugs :-)

[cperkinsintel]

ping to reviewers.
There is a separate ticket opened for the conformance issue mentioned in the comments. But the fix here is still needed. Without it, we can't instantiate unspecialized spans at all.