[SYCL][L0] Support DPC++ extension Queue Order Properties. #3396

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Merged

pvchupin merged 20 commits into intel:sycl from rbegam:in-order-queue

Apr 1, 2021

Contributor

rbegam commented Mar 23, 2021 •

edited

Loading

Adds in-order queue semantics.
Reverts [SYCL] Temporarily restore event deps in in-order queues for Level Zero #3188

Signed-off-by: rbegam rehana.begam@intel.com

rbegam requested review from smaslov-intel and a team as code owners

March 23, 2021 02:18

smaslov-intel reviewed

View reviewed changes

sycl/include/CL/sycl/detail/pi.h Outdated Show resolved Hide resolved

smaslov-intel reviewed

View reviewed changes

sycl/source/detail/queue_impl.hpp Outdated Show resolved Hide resolved

smaslov-intel reviewed

View reviewed changes

sycl/plugins/level_zero/pi_level_zero.hpp Outdated Show resolved Hide resolved

smaslov-intel reviewed

View reviewed changes

sycl/plugins/level_zero/pi_level_zero.hpp Outdated Show resolved Hide resolved

smaslov-intel reviewed

View reviewed changes

sycl/plugins/level_zero/pi_level_zero.hpp Outdated Show resolved Hide resolved

smaslov-intel reviewed

View reviewed changes

sycl/plugins/level_zero/pi_level_zero.hpp Outdated Show resolved Hide resolved

smaslov-intel reviewed

View reviewed changes

sycl/plugins/level_zero/pi_level_zero.cpp Outdated Show resolved Hide resolved

smaslov-intel reviewed

View reviewed changes

sycl/plugins/level_zero/pi_level_zero.cpp Show resolved Hide resolved

smaslov-intel reviewed

View reviewed changes

sycl/plugins/level_zero/pi_level_zero.hpp Outdated Show resolved Hide resolved

smaslov-intel reviewed

View reviewed changes

sycl/plugins/level_zero/pi_level_zero.hpp Outdated Show resolved Hide resolved

smaslov-intel reviewed

View reviewed changes

sycl/plugins/level_zero/pi_level_zero.cpp Outdated Show resolved Hide resolved

smaslov-intel reviewed

View reviewed changes

sycl/plugins/level_zero/pi_level_zero.cpp Outdated

    
            @@ -2138,6 +2157,7 @@ pi_result piQueueCreate(pi_context Context, pi_device Device,
          
                try {

                  *Queue =

                      new _pi_queue(ZeCommandQueue, Context, Device, ZeCommandListBatchSize);

                  (*Queue)->PiQueueProperties = Properties;

Contributor

smaslov-intel Mar 24, 2021

add this to constructor instead

Contributor

asudarsa Mar 26, 2021

We call new _pi_queue in piextQueueCreateWithNativeHandle as well. But there is no Properties coming into that function. What do we do there? Thanks.

Contributor

smaslov-intel Mar 26, 2021

We will have to choose something as a default, I guess OOO since that is the default SYCL's choice. If needed we could add an extension for users' to specify properties with the interop API later on.

Contributor

smaslov-intel Mar 26, 2021

Please do have it in the constructor.

Contributor Author

rbegam Mar 29, 2021

sorry, missed this comment, done.

rbegam force-pushed the in-order-queue branch from 7a9557c to 1fb830a Compare

March 24, 2021 22:55

Contributor

smaslov-intel commented Mar 25, 2021

We need to record/wait for submissions to the immediate command-list of the queue in piMemBufferCreate/piMemImageCreate

asudarsa reviewed

View reviewed changes

sycl/plugins/level_zero/pi_level_zero.cpp Outdated Show resolved Hide resolved

smaslov-intel suggested changes

View reviewed changes

Contributor

smaslov-intel left a comment

I've noted something that I'd asked you to change. Please don't use "amend" with the new commits, such that I can review incremental changes only.

rbegam force-pushed the in-order-queue branch from e327b24 to cd63fdd Compare

March 29, 2021 17:11

smaslov-intel reviewed

View reviewed changes

sycl/source/detail/scheduler/commands.cpp Outdated Show resolved Hide resolved

rbegam force-pushed the in-order-queue branch from cd63fdd to 826a362 Compare

March 29, 2021 17:49

smaslov-intel suggested changes

View reviewed changes

Contributor

smaslov-intel left a comment

few more comments

sycl/plugins/level_zero/pi_level_zero.cpp Outdated Show resolved Hide resolved

sycl/plugins/level_zero/pi_level_zero.cpp Show resolved Hide resolved

sycl/plugins/level_zero/pi_level_zero.cpp Outdated Show resolved Hide resolved

rbegam changed the title ~~[WIP][SYCL][L0] support DPC++ extension Queue Order Properties.~~ [SYCL][L0] support DPC++ extension Queue Order Properties.

rbegam force-pushed the in-order-queue branch from 70608cd to 8a4ecd6 Compare

March 29, 2021 23:24

smaslov-intel reviewed

View reviewed changes

sycl/plugins/level_zero/pi_level_zero.cpp Outdated Show resolved Hide resolved

smaslov-intel reviewed

View reviewed changes

sycl/plugins/level_zero/pi_level_zero.cpp Outdated Show resolved Hide resolved

Contributor Author

rbegam commented Mar 31, 2021 •

edited

Loading

the only lit-fail for SIMD/vadd.cpp is unrelated to this change. I've talked with @DenisBakhvalov and he has uploaded a fix here: #3457.
as soon as that PR is merged, lit_with_cuda, sycl-ubu-x64-pr and sycl-win-x64-pr should pass.

smaslov-intel previously approved these changes

View reviewed changes

Contributor Author

rbegam commented Apr 1, 2021 •

edited

Loading

there are 4 fails (3 time-outs and one ESIMD fail) for windows precommit testing for OpenCL which says
"Both exp and ref have failure cases for PI_OPENCL".

for level_zero both linux and windows precommit tests pass. I am restarting the test.
for the ESIMD, I think this requires the same correcting as #3457.


          [WIP][SYCL][L0] support DPC++ extension Queue Order Properties.

e81aa64

Signed-off-by: rbegam <rehana.begam@intel.com>

rbegam added 17 commits

March 31, 2021 18:49


          clang formatted.

820ca0a

Signed-off-by: rbegam <rehana.begam@intel.com>


          modified queue members.

089c920

Signed-off-by: rbegam <rehana.begam@intel.com>


          updated createAndRetainPiZeEventList().

24cc182

Signed-off-by: rbegam <rehana.begam@intel.com>


          commants removed, clang formatted

f532568

Signed-off-by: rbegam <rehana.begam@intel.com>


          clang formatted.

03d7776

Signed-off-by: rbegam <rehana.begam@intel.com>


          re-arranged some codes.

e71002f

Signed-off-by: rbegam <rehana.begam@intel.com>


          clang formatted.

5a1d339

Signed-off-by: rbegam <rehana.begam@intel.com>


          added lock

79c3594

Signed-off-by: rbegam <rehana.begam@intel.com>


          code cleanup.

81b81fa

Signed-off-by: rbegam <rehana.begam@intel.com>


          clang formatted, comments added.

f9d747e

Signed-off-by: rbegam <rehana.begam@intel.com>


          minor changes.

408b7da

Signed-off-by: rbegam <rehana.begam@intel.com>


          clang formatted.

4a2df5c

Signed-off-by: rbegam <rehana.begam@intel.com>


          restore accidentally deleted code.

6a4bb7b

Signed-off-by: rbegam <rehana.begam@intel.com>


          fix initialization of lastCommandEvent.

7b8c809

Signed-off-by: rbegam <rehana.begam@intel.com>


          adds new locks.

727b8e6

Signed-off-by: rbegam <rehana.begam@intel.com>


          fix the bug.

11d3408

Signed-off-by: rbegam <rehana.begam@intel.com>


          clang formatted.

7764d90

Signed-off-by: rbegam <rehana.begam@intel.com>

rbegam dismissed smaslov-intel’s stale review via

7764d90

April 1, 2021 02:12

rbegam force-pushed the in-order-queue branch from f1050f4 to 7764d90 Compare

April 1, 2021 02:12

smaslov-intel approved these changes

View reviewed changes

bader requested a review from asudarsa

April 1, 2021 17:42

bader changed the title ~~[SYCL][L0] support DPC++ extension Queue Order Properties.~~ [SYCL][L0] Support DPC++ extension Queue Order Properties.

asudarsa approved these changes

View reviewed changes

Contributor

asudarsa left a comment

Looks good to me. Thanks

pvchupin merged commit 5bc3cb2 into intel:sycl

pvchupin pushed a commit that referenced this pull request


          Revert "[SYCL][L0] Support DPC++ extension Queue Order Properties. (#…

8449ebc

…3396)"

This reverts commit 5bc3cb2.

pvchupin mentioned this pull request

Revert "[SYCL][L0] Support DPC++ extension Queue Order Properties." #3479

Merged

pvchupin pushed a commit that referenced this pull request


          Revert "[SYCL][L0] Support DPC++ extension Queue Order Properties. (#…

3160de0

…3396)" (#3479)

This reverts commit 5bc3cb2 
done in PR#3396 due to issues found.

rbegam added a commit to rbegam/llvm that referenced this pull request


          Revert "Revert "[SYCL][L0] Support DPC++ extension Queue Order Proper…

ef8b072

…ties. (intel#3396)" (intel#3479)"

This reverts commit 3160de0.

rbegam mentioned this pull request

[SYCL][L0] Support DPC++ extension Queue Order Properties. #3483

Merged

pvchupin pushed a commit that referenced this pull request


          [SYCL][L0] Support DPC++ extension Queue Order Properties. (#3483)

50005c7

Return back "[SYCL][L0] Support DPC++ extension Queue Order Properties. (#3396)" reverted earlier in (#3479)
Added fix for the segfault happening due to wait on an already released event.
The related test SYCL/InorderQueue/in_order_event_release.cpp is added in llvm-test-suite.

Signed-off-by: rbegam rehana.begam@intel.com

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet