Should types with explicit width be used in function used by kernel in SSCP, instead of e.g. size_t ?

[fxzjshm]

For functions like __hipsycl_sscp_get_local_id_x

https://github.com/OpenSYCL/OpenSYCL/blob/cd2c72f459dd449ca40a61b6b0c9671a06d7d1c7/include/hipSYCL/sycl/libkernel/sscp/builtins/core.hpp#L35

when kernel is compiled to hcf size_t has one kind of width e.g. uint64;
but when libkernel-sscp-*-core.bc is compiled to some GPU architecture (other than nvptx64, amdgcn or spir64, will upstream once done) , they may have uint32 defined as size_t,
then linking two LLVM IRs leads to expressions like this:

; Function Attrs: alwaysinline mustprogress nofree nosync nounwind readnone willreturn
define internal i32 @__hipsycl_sscp_get_local_id_x() unnamed_addr #12 {
  %1 = tail call i32 @llvm.somevendor.read.sreg.tid.x()
  ret i32 %1
}

; in kernel, reading local id
%17 = tail call i64 bitcast (i32 ()* @__hipsycl_sscp_get_local_id_x to i64 ()*)() #6

Don't know whether this is expected, but I think such bitcast seems strange, so I suggest __hipsycl_uint64 instead of size_t here.

[illuhad]

I agree, all of the SSCP builtin interface should rely on the __hipsycl fixed width types for this reason. This looks like an oversight.

(other than nvptx64, amdgcn or spir64, will upstream once done)

Looking forward to your work! :)

[fxzjshm]

OK, I'm going to create a PR for this

[fxzjshm]

-> #1076

Looking forward to your work! :)

Thanks!

[al42and]

On a related note: I have noticed that sometimes the use of 64-bit integers for indexes generate more arithmetic ops compared to native CUDA (threadIdx and co. return int32, they are converted to int64 to compute global_linear_id, then converted back to int32 in user code when int is used for it, CUDA-style).

DPC++ has special compiler flag for it, -fsycl-id-queries-fit-in-int, and, from the user code, __builtin_assume seems to help the compiler deduce the ranges of values involved and keep everything int32.

Not a big problem per se (never had been a bottleneck for us 😄), but might be something to consider in the scope of this discussion.

[illuhad]

If introduce another macro, consider someone compiled SSCP stage 1 with macro set to e.g. uint32 then deployed binary to servers, but on server they compiled libkernel with macro set to e.g. uint64, I'm afraid they may encounter the same problem on stage 2 as what I've met.

I agree, I'm not suggesting to change the builtin interface, but to work entirely in the SYCL headers. For example:

namespace sycl {

template<int Dim>
class id {
  ....
private
#ifdef HIPSYCL_SMALL_ID_QUERIES
   detail::device_array<unsigned, Dim> _data;
#else
   detail::device_array<std::size_t, Dim> _data;
#endif
};

}

auto make_work_item_local_id() {
#ifdef HIPSYCL_SMALL_ID_QUERIES
   __builtin_assume(__hipsycl_lid_x < ...);
  ...
#endif
   return id{__hipsycl_lid_x, __hipsycl_lid_y, __hipsycl_lid_z};
}

But this is not really related anymore to your PR :)

[fxzjshm]

Oh I misunderstood...

And maybe need to take care of other places like

https://github.com/OpenSYCL/OpenSYCL/blob/caee5569280569c8dcc91fd8439fcf583924a11c/include/hipSYCL/glue/generic/hiplike/hiplike_kernel_launcher.hpp#L189

[illuhad]

Because all device backends in the end just use the __hipsycl_* macros (like __hipsycl_lid_x) as in your code snippet, maybe it would even be sufficient to just adapt those..

[fxzjshm]

I mean the int gid here, maybe int64 gid or uint64 gid if HIPSYCL_SMALL_ID_QUERIES is not defined I think.
(Maybe someone want to reduce on > 2^32 numbers?)

[illuhad]

I'm currently working on a first step to rewriting reductions anyway, so all that existing code is planned to be replaced eventually. But you're right, just using int here is potentially a bug.