Fix the 2D buffers issues with Xilinx devices

[Victorlzd]

Fix the 2D buffers issues with Xilinx devices by calling clEnqueueBuffer instead of clEnqueueBufferRect when the whole buffer is copied

On branch fix_2D_buffer
Changes to be committed:
modified: source/detail/memory_manager.cpp

Commiter victorlzd victorlezaud@gmail.com

[agozillon]

Other than the comments that are mostly just some indentation issues and related to the removal of UseExclusiveQueue (which may have been an accidental removal when rebasing), I have a couple of requests if at all possible:

• @Victorlzd Could you please add [SYCL][Runtime] to the front of the commit so it's easy to find, since every now and again we'll get a massive deluge of upstream Clang/LLVM commits. Where possible add the [SYCL] prefix to commits and if you can add more tags like [Runtime] or [Driver] etc. to make it more specific what the commit touches on that's even better!
• And for @keryell when this gets merged in, if it's possible could it be merged into next rather than master as we'll have different histories on Next and Master otherwise I think.. but it shouldn't have any conflict in either branch at the moment

Thank you for your work!

[Victorlzd]

I agree with all your comments.
I will do all the correction and repush the branch.

[agozillon]

I agree with all your comments.
I will do all the correction and repush the branch.

Thank you very much! It was probably because the master branch was updated a little while ago to reflect changes in the main upstream SYCL implementation, which I'll be doing again next week most likely.

[agozillon]

Looking much better now, but I think the style is still a little off at a few locations e.g. "if( SrcSize[0] -> if (SrcSize[0]" and some extra white space here and there. These are some pedantic style changes on my part to keep it inline with the existing style in place though.. so if @keryell is fine without the suggested changes to the patch I'm fine with it.

Other than that the commit message is much improved, although it should be for example:

[SYCL][Runtime] Whatever My Commit Title Is

More commit details here

That way the tags and title can easily be skimmed over when reading through changes! Sorry, looking back I wasn't entirely clear on that.

[keryell]

I think you can simplify the code a little bit.

[keryell]

Making progress...
Please take into account @agozillon review so I do not to do it again...
please configure correctly your environment: your commit is a mess of TAB and spaces, trailing spaces...

[keryell]

That looks better.
There are still a few spaces missing...

[keryell]

Almost done.
Can you rebase the branch on top of sycl/unified/next?
It will simplify the history and also resolve the conflict due to the recent change in the runtime naming.

[agozillon]

Also, if you have a test case showcasing this working, I would appreciate you adding this in a seperate pull request!

Generally if you make a change altering some behavior I like to have a new test for it added to simple_tests.. although I suppose in this case its probably tested inside the existing Intel SYCL test suite and can be checked via: make check-sycl

@keryell out of interest, how easy would it be to run the same test-suite on each pull request that the Intel repository currently runs? Would be nice to see if any changes break existing functionality when people pull request in.. similar to triSYCL! Even if we can't test FPGAs using the same method.

[keryell]

@agozillon For the continuous integration aspect, I created #53

[keryell]

Thanks.