[SYCL] Fix memory deallocation size, fix 2 CTS test fails on Windows

[v-klochkov]

Signed-off-by: Vyacheslav N Klochkov vyacheslav.n.klochkov@intel.com

[mibintc]

Nice work! Can you add test cases for this? Is this unique to Windows? Why does it not fail on Linux?

[v-klochkov]

Thank you for the review and comments. I added a test case to buffer.cpp LIT test.

I suppose it did not fail on Linux because one of these
a) allocation is aligned to bigger number of bytes, thus no need to create a copy of the buffer, i.e. the original memory can be re-used.
b) I don't know, but Linux might just not care about the argument telling how many elements are deallocated.

[mibintc]

Thanks a lot for fixing this. I saw similar problems on OpenCL_interop_constructors and I hope it fixes those failures too.

[keryell]

What about:

auto constexpr AllocatorValueSize = sizeof(allocator_value_type_t<AllocatorT>);
return (get_size() + AllocatorValueSize - 1)/AllocatorValueSize;

?

[v-klochkov]

Well, this looks better if we do not care about possible overflow on 32-bit targets in this expression: (get_size() + AllocatorValueSize - 1).

The current version of code may produce two DIV operations (depending on compiler), but that can be fixed easily by having this modified version of the original code (stored the result of get_size() to a var):

auto AllocatorValueSize = sizeof(allocator_value_type_t<AllocatorT>);
auto Size = get_size();
auto AllocationCount = Size / AllocatorValueSize;
return AllocationCount + (Size % AllocatorValueSize) ? 1 : 0;

Usually compilers can optimize two sequential divs: (A / B) and ( A % B) into one asm division operation.

Do you still recommend using your version? or the modified variant showed above?

[keryell]

If we have an overflow on get_size() + AllocatorValueSize - 1 we have some other problems to worry first... :-)
The problem is also the ?: that might be inefficient. Anyway, I hope that this is just replaced by some bit operations in my constexpr version.
But at the first place I am surprised that get_count() is computed from get_size() and not the opposite... Because you are focused on the allocated memory and not the number of elements of the object seen by the user (a buffer of n objects T for example)?
Anyway, I hope these functions are not on the critical path of a real application...

[v-klochkov]

Ok, I changed the code to your version. Thank you.
Regarding keeping the number of elements allocated instead of the number of bytes..., I do not change it in this patch. It would require a separate more elaborate and risky change-set, talk to original author, and a good reason for fix.