[SYCL][ESIMD] Add more compile time checks to rdregion and wrregion API

[fineg74]

No description provided.

[fineg74]

AMD/HIP test failures are not related to the change

[v-klochkov]

I understand this change as ESIMD developer and it is correct, but...
It is a very user-unfriendly fix. It adds extra constraints to the code that could function without harm to the program execution (GPU driver promised to handle correctly (skip) the reads/writes out of esimd register scope).
If there are programs with such bad reads/writes they would fail at compilation with very odd error not giving information about what needs to be fixed.

If add such constraints, then they should be added in a user friendly manner. I.e. at the user's functions calling __esimd_rdregion/wrregion or at the places where the simd_view objects are created to prohibit potential reads/writes beyond simd object scope.

[fineg74]

I understand this change as ESIMD developer and it is correct, but... It is a very user-unfriendly fix. It adds extra constraints to the code that could function without harm to the program execution (GPU driver promised to handle correctly (skip) the reads/writes out of esimd register scope). If there are programs with such bad reads/writes they would fail at compilation with very odd error not giving information about what needs to be fixed.

If add such constraints, then they should be added in a user friendly manner. I.e. at the user's functions calling __esimd_rdregion/wrregion or at the places where the simd_view objects are created to prohibit potential reads/writes beyond simd object scope.

Yes, originally, these constraints were introduced for emulator that used to crash when read/write out of bounds was performed, but I believe it still has some value as it would allow to uncover some hidden bugs that would be impossible to detect otherwise. I don' have strong opinion about that though.

[v-klochkov]

Please write a detailed comment to these 2 functions and their params. Otherwise, it is really difficult (or impossible) to understand what they verify.

[fineg74]

Added some comments

[v-klochkov]

Thank you. It is still difficult to understand why the constraint is such: Width == M || ((M / Width) - 1) * VStride + (Width - 1) * Stride < N. It would help a lot if the pseudo-code would be copied from __esimd_rdregion/wrregion to the commend of these check functions, e.g.;
// This intrinsic computes a vector Result:
//
// \code{.cpp}
// uint16_t EltOffset = Offset / sizeof(T);
// assert(Offset % sizeof(T) == 0);
//
// int NumRows = M / Width;
// assert(M % Width == 0);
//
// int Index = 0;
// for (int i = 0; i < NumRows; ++i) {
// for (int j = 0; j < Width; ++j) {
// Result[Index++] = Input[i * VStride + j * Stride +
// EltOffset];
// }
// }
// \endcode

[fineg74]

Done

[v-klochkov]

Can you do the same for __esimd_wrregion please?
Perhaps, fix the comment in both places too - comment does not say that the write is skipped if mask is 0.

[v-klochkov]

Sorry, for confusion. the pseudo-code for __esimd_wrregion is correct. Please just copy it to check function here.
The __esimd_rdregion does not have a mask operand, thus is is all right in the pseudo-code for rdregion.

[fineg74]

Done