Skip to content

[SYCL] Fix sync of host task vs kernel for in-order queue#5551

Merged
bader merged 6 commits intointel:syclfrom
KseniyaTikhomirova:kernel_sync_host
Feb 15, 2022
Merged

[SYCL] Fix sync of host task vs kernel for in-order queue#5551
bader merged 6 commits intointel:syclfrom
KseniyaTikhomirova:kernel_sync_host

Conversation

@KseniyaTikhomirova
Copy link
Contributor

@KseniyaTikhomirova KseniyaTikhomirova commented Feb 11, 2022

Fix + unit test SchedulerTest.InOrderQueueHostTaskDepsExt

@KseniyaTikhomirova
Fix sync of host task vs kernel for in-order queue
fa6c774 
Signed-off-by: Tikhomirova, Kseniya <kseniya.tikhomirova@intel.com>
@KseniyaTikhomirova
Tiny code fixes: comments and extra header removal
ecf2fdb 
Signed-off-by: Tikhomirova, Kseniya <kseniya.tikhomirova@intel.com>
@KseniyaTikhomirova KseniyaTikhomirova requested a review from a team as a code owner February 11, 2022 12:37
@KseniyaTikhomirova
Copy link
Contributor Author

KseniyaTikhomirova commented Feb 11, 2022

test reasonably fails, will work on test fix or redesign.

@KseniyaTikhomirova
Redesign test to verify single finalizeHandler function
b3f0555 
Signed-off-by: Tikhomirova, Kseniya <kseniya.tikhomirova@intel.com>
@KseniyaTikhomirova
Add comment for queue change
968c8b0 
Signed-off-by: Tikhomirova, Kseniya <kseniya.tikhomirova@intel.com>
alexbatashev
alexbatashev reviewed Feb 14, 2022
View reviewed changes
@KseniyaTikhomirova
Code-review: fix test header and namespace
e14e03e 
Signed-off-by: Tikhomirova, Kseniya <kseniya.tikhomirova@intel.com>
s-kanaev
s-kanaev previously approved these changes Feb 14, 2022
View reviewed changes
Copy link
Contributor

@s-kanaev s-kanaev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good overall

alexbatashev
alexbatashev previously approved these changes Feb 14, 2022
View reviewed changes
@KseniyaTikhomirova
Code-review: fix grammar in comments
e05636a 
Signed-off-by: Tikhomirova, Kseniya <kseniya.tikhomirova@intel.com>
s-kanaev
s-kanaev approved these changes Feb 15, 2022
View reviewed changes
Copy link
Contributor

@s-kanaev s-kanaev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@bader bader requested a review from alexbatashev February 15, 2022 13:54
@bader bader merged commit c7ba937 into intel:sycl Feb 15, 2022
@bader
Copy link
Contributor

bader commented Feb 15, 2022

@KseniyaTikhomirova, post-commit testing failed with

/home/runner/work/llvm/llvm/src/sycl/unittests/scheduler/InOrderQueueSyncCheck.cpp:20:7: error: 'LimitedHandler' has virtual functions but non-virtual destructor [-Werror,-Wnon-virtual-dtor]
class LimitedHandler {
^
/home/runner/work/llvm/llvm/src/sycl/unittests/scheduler/InOrderQueueSyncCheck.cpp:33:7: error: 'LimitedHandlerSimulation' has virtual functions but non-virtual destructor [-Werror,-Wnon-virtual-dtor]
class LimitedHandlerSimulation : public LimitedHandler {
^
2 errors generated.

Could you fix this issue, please?

KseniyaTikhomirova added a commit to KseniyaTikhomirova/llvm that referenced this pull request Feb 15, 2022
@KseniyaTikhomirova
[SYCL] Fix warning for InOrderQueueSyncCheck unit test build
943628f 
fixes post-commit check for intel#5551

Signed-off-by: Tikhomirova, Kseniya <kseniya.tikhomirova@intel.com>
@KseniyaTikhomirova KseniyaTikhomirova mentioned this pull request Feb 15, 2022
Merged
@KseniyaTikhomirova
Copy link
Contributor Author

KseniyaTikhomirova commented Feb 15, 2022

@KseniyaTikhomirova, post-commit testing failed with

/home/runner/work/llvm/llvm/src/sycl/unittests/scheduler/InOrderQueueSyncCheck.cpp:20:7: error: 'LimitedHandler' has virtual functions but non-virtual destructor [-Werror,-Wnon-virtual-dtor] class LimitedHandler { ^ /home/runner/work/llvm/llvm/src/sycl/unittests/scheduler/InOrderQueueSyncCheck.cpp:33:7: error: 'LimitedHandlerSimulation' has virtual functions but non-virtual destructor [-Werror,-Wnon-virtual-dtor] class LimitedHandlerSimulation : public LimitedHandler { ^ 2 errors generated.

Could you fix this issue, please?

fixed in this PR #5577, thanks

bader pushed a commit that referenced this pull request Feb 15, 2022
@KseniyaTikhomirova
[SYCL] Fix warning for InOrderQueueSyncCheck unit test build (#5577)
d4412d7 
Fixes post-commit check for #5551

Signed-off-by: Tikhomirova, Kseniya <kseniya.tikhomirova@intel.com>
alexbatashev added a commit to alexbatashev/llvm that referenced this pull request Feb 23, 2022
@alexbatashev
Merge branch 'refactor_existing_workflows' of github.com:alexbatashev…
b40a1e1 
…/llvm into refactor_existing_workflows

* 'refactor_existing_workflows' of github.com:alexbatashev/llvm: (2051 commits)
  [SYCL][L0] Honor property::queue::enable_profiling (intel#5543)
  [SYCL][CI] Enable sccache on Windows (intel#5589)
  [SYCL][Doc] Move internal design docs (intel#5556)
  [sycl-post-link] Initialize the integer Value variable (intel#5585)
  [CI] Fix nightly builds (intel#5584)
  [SYCL][L0] Fix use of copy-engines in L0 interop queue (intel#5579)
  Update OpenCL headers tag to dcd5bed (intel#5575)
  [SYCL] Fix warning for InOrderQueueSyncCheck unit test build (intel#5577)
  [SYCL][HIP] Remove arch requirement for running lit tests (intel#5253)
  [SYCL][L0] Fix timestamp calculation (in ns) (intel#5555)
  [SYCL] Fix sync of host task vs kernel for in-order queue (intel#5551)
  [sycl-post-link] Add a check for device globals with device_image_scope (intel#5517)
  [SYCL] Fix SYCL Kernel Body Check (intel#5546)
  [SYCL] Add support for SYCL 2020 in class group (intel#5447)
  Fix tests after 1c729d7 Use align attribute for kernel pointer arg alignment
  Fix tests after 18834dc Mark pointer-typed kernel arguments as ABI aligned
  [CI] Add experimental Windows build to GitHub Actions nightly (intel#5560)
  [sycl-post-link][NFC] Address clang-tidy concerns in the sycl-post-link (intel#5552)
  Fix lit test after changes in Clang
  Improve backward compatibility for DISubRange
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@sergey-semenov sergey-semenov Awaiting requested review from sergey-semenov sergey-semenov is a code owner automatically assigned from intel/llvm-reviewers-runtime

2 more reviewers

@alexbatashev alexbatashev alexbatashev approved these changes

@s-kanaev s-kanaev s-kanaev approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@KseniyaTikhomirova @bader @alexbatashev @s-kanaev