[SYCL] Fix performance regression in parallel_for with using id class

[aobolensk]

In #5118 conversion for user defined types which are implicitly converted from sycl::items was added. It caused regression in case when sycl::id type is used, because with this change it converts to sycl::item but in fact it is not needed. That is why sycl::id class case should be checked explicitly.

[vladimirlaz]

@aobolensk is it possible to add test? Generate some IR and check that type is actually used?

[keryell]

This seems useful but please all the _t and _v versions of the type traits.

[aobolensk]

This seems useful but please all the _t and _v versions of the type traits.

Done

[bader]

@keryell, does it look good now?

[keryell]

We are using at least C++17, right?

Suggested change

	using type = typename std::conditional_t<
	std::is_same<id<Dims>, LambdaArgType>::value, LambdaArgType,
	typename std::conditional_t<
	std::is_convertible<nd_item<Dims>, LambdaArgType>::value,
	nd_item<Dims>,
	typename std::conditional_t<
	std::is_convertible<item<Dims>, LambdaArgType>::value,
	item<Dims>, LambdaArgType>>>;
	using type = typename std::conditional_t<
	std::is_same_v<id<Dims>, LambdaArgType>, LambdaArgType,
	typename std::conditional_t<
	std::is_convertible_v<nd_item<Dims>, LambdaArgType>,
	nd_item<Dims>,
	typename std::conditional_t<
	std::is_convertible_v<item<Dims>, LambdaArgType>,
	item<Dims>, LambdaArgType>>>;

Perhaps you can try whether you can remove the typename and see whether it still compiles.

[aobolensk]

I have tried to replace std::is_same with std::is_same_v too, but it leads to basic_tests/stdcpp_compat.cpp failure (https://github.com/intel/llvm/runs/4948816096?check_suite_focus=true)

[keryell]

By looking at the compilation line in your CI test I can see: "/__w/llvm/llvm/build/bin/clang" "--driver-mode=g++" "-Xsycl-target-backend=amdgcn-amd-amdhsa" "--offload-arch=gfx906" "-D_GLIBCXX_ASSERTIONS=1" "-std=c++14"
C++14 whaaaat? :-)
@bader do you still have some old C++14 while SYCL 2020 is requiring at least C++17?

[bader]

Yes.
https://github.com/intel/llvm/blob/sycl/sycl/test/basic_tests/stdcpp_compat.cpp#L13 - as you can see this comment says that some features can be unavailable. We are trying to transition old codes written in SYCL-1.2.1 softly.
New code requiring c++17 features can be added under #ifdef __cplusplus >= 201703L.

[keryell]

Perhaps this is a good opportunity to extend https://github.com/intel/llvm/blob/54ede407edeb93b7e9334d1e725f48bf981b2965/sycl/include/CL/sycl/detail/stl_type_traits.hpp with a few _v ones in the meantime?

[aobolensk]

@bader do we need to add missing _v functions to stl_type_traits.hpp or we are going to leave it as it is now?

[bader]

do we need to add missing _v functions to stl_type_traits.hpp

@aobolensk, I think this is what @keryell requests. Do you have any objections?

[aobolensk]

do we need to add missing _v functions to stl_type_traits.hpp

@aobolensk, I think this is what @keryell requests. Do you have any objections?

@bader No objections. I will add it.

[keryell]

Thanks for the simplifications!