[UR][L0] Propagate errors from `USMAllocationMakeResident` #11312

0x12CC · 2023-09-26T20:14:21Z

Update the UR tag to include a bug fix in L0.

jandres742

thanks @0x12CC . Please move this PR to the UR repo, since code for L0 is now there. instructions for contribution to follow but please check what we are doing here oneapi-src/unified-runtime#901

This change ensures that USM allocation APIs don't return `UR_RESULT_SUCCESS` when an error occurs within `USMAllocationMakeResident`. Signed-off-by: Michael Aziz <michael.aziz@intel.com>

Signed-off-by: Michael Aziz <michael.aziz@intel.com>

0x12CC · 2023-10-16T20:38:08Z

@jandres742, @intel/dpcpp-l0-pi-reviewers, could anyone please review this PR?

jandres742 · 2023-10-16T20:51:27Z

thanks @0x12CC . Please confirm whether the failures are related to your changes or not.

Signed-off-by: Michael Aziz <michael.aziz@intel.com>

kbenzie · 2023-10-17T15:26:31Z

thanks @0x12CC . Please confirm whether the failures are related to your changes or not.

This is also affecting #11344 when bumps UR to a newer commit than this PR. Any idea what's causing this? Was the issue somehow hidden before?

0x12CC · 2023-10-17T17:13:30Z

@jandres742, @kbenzie, there are two sets of failures:

Failed Tests (25):
  SYCL :: Matrix/Legacy/XMX8/element_wise_all_ops_bf16.cpp
  SYCL :: Matrix/Legacy/XMX8/element_wise_all_ops_half.cpp
  SYCL :: Matrix/Legacy/XMX8/element_wise_irreg_sum_rows.cpp
  SYCL :: Matrix/Legacy/XMX8/element_wise_ops.cpp
  SYCL :: Matrix/Legacy/XMX8/joint_matrix_bf16.cpp
  SYCL :: Matrix/Legacy/XMX8/joint_matrix_bfloat16.cpp
  SYCL :: Matrix/Legacy/XMX8/joint_matrix_half.cpp
  SYCL :: Matrix/Legacy/XMX8/joint_matrix_ss_int8.cpp
  SYCL :: Matrix/Legacy/XMX8/joint_matrix_su_int8.cpp
  SYCL :: Matrix/Legacy/XMX8/joint_matrix_us_int8.cpp
  SYCL :: Matrix/Legacy/XMX8/joint_matrix_uu_int8.cpp
  SYCL :: Matrix/XMX8/element_wise_abc.cpp
  SYCL :: Matrix/XMX8/element_wise_all_ops_half.cpp
  SYCL :: Matrix/XMX8/element_wise_ops.cpp
  SYCL :: Matrix/XMX8/get_coord_float_matC.cpp
  SYCL :: Matrix/XMX8/get_coord_int8_matA.cpp
  SYCL :: Matrix/XMX8/joint_matrix_all_sizes.cpp
  SYCL :: Matrix/XMX8/joint_matrix_apply_bf16.cpp
  SYCL :: Matrix/XMX8/joint_matrix_bfloat16.cpp
  SYCL :: Matrix/XMX8/joint_matrix_bfloat16_array.cpp
  SYCL :: Matrix/XMX8/joint_matrix_half.cpp
  SYCL :: Matrix/XMX8/joint_matrix_ss_int8.cpp
  SYCL :: Matrix/XMX8/joint_matrix_su_int8.cpp
  SYCL :: Matrix/XMX8/joint_matrix_us_int8.cpp
  SYCL :: Matrix/XMX8/joint_matrix_uu_int8.cpp

Failed Tests (1):
  SYCL :: DeviceLib/built-ins/ext_native_math.cpp

The ext_native_math failure is related to #11562. I'm not sure what's causing the matrix test failures. I don't think they're related to my change in UR but they could be caused by any other commit included in this version update.

jandres742 · 2023-10-17T18:05:38Z

@jandres742, @kbenzie, there are two sets of failures:

Failed Tests (25):
  SYCL :: Matrix/Legacy/XMX8/element_wise_all_ops_bf16.cpp
  SYCL :: Matrix/Legacy/XMX8/element_wise_all_ops_half.cpp
  SYCL :: Matrix/Legacy/XMX8/element_wise_irreg_sum_rows.cpp
  SYCL :: Matrix/Legacy/XMX8/element_wise_ops.cpp
  SYCL :: Matrix/Legacy/XMX8/joint_matrix_bf16.cpp
  SYCL :: Matrix/Legacy/XMX8/joint_matrix_bfloat16.cpp
  SYCL :: Matrix/Legacy/XMX8/joint_matrix_half.cpp
  SYCL :: Matrix/Legacy/XMX8/joint_matrix_ss_int8.cpp
  SYCL :: Matrix/Legacy/XMX8/joint_matrix_su_int8.cpp
  SYCL :: Matrix/Legacy/XMX8/joint_matrix_us_int8.cpp
  SYCL :: Matrix/Legacy/XMX8/joint_matrix_uu_int8.cpp
  SYCL :: Matrix/XMX8/element_wise_abc.cpp
  SYCL :: Matrix/XMX8/element_wise_all_ops_half.cpp
  SYCL :: Matrix/XMX8/element_wise_ops.cpp
  SYCL :: Matrix/XMX8/get_coord_float_matC.cpp
  SYCL :: Matrix/XMX8/get_coord_int8_matA.cpp
  SYCL :: Matrix/XMX8/joint_matrix_all_sizes.cpp
  SYCL :: Matrix/XMX8/joint_matrix_apply_bf16.cpp
  SYCL :: Matrix/XMX8/joint_matrix_bfloat16.cpp
  SYCL :: Matrix/XMX8/joint_matrix_bfloat16_array.cpp
  SYCL :: Matrix/XMX8/joint_matrix_half.cpp
  SYCL :: Matrix/XMX8/joint_matrix_ss_int8.cpp
  SYCL :: Matrix/XMX8/joint_matrix_su_int8.cpp
  SYCL :: Matrix/XMX8/joint_matrix_us_int8.cpp
  SYCL :: Matrix/XMX8/joint_matrix_uu_int8.cpp

Failed Tests (1):
  SYCL :: DeviceLib/built-ins/ext_native_math.cpp

The ext_native_math failure is related to #11562. I'm not sure what's causing the matrix test failures. I don't think they're related to my change in UR but they could be caused by any other commit included in this version update.

thanks @0x12CC . for the 25 failures, then two things to do:

could you run one of the test with and without your changes in the UR, on top of the commit you are trying to update here in the CMakeLists? that would help us knowing whether the failures are specific to your changes.
if they are not specific to your changes, I would suggest then to test with different commits between the prev version update and the one being updated there, to find which UR update is causing the problems.

jandres742 · 2023-10-19T00:05:40Z

@kbenzie

I was not able to reproduce the failures in a Arc system. So to confirm where the error might be coming from, I created draft PRs for the updates on each of the UR commits between current and the one we are trying here:

testing update in #11587

commit 4954850f338241d72d506ebcb7162d61afef2924
Merge: 0677296 e5d6a91
Author: Kenneth Benzie (Benie) <k.benzie@codeplay.com>
Date:   Tue Oct 10 11:46:13 2023 +0100

    Merge pull request #906 from 0x12CC/l0_usm_error_checking

    [UR][L0] Propagate errors from `USMAllocationMakeResident`

testing update in #11586

commit 484cd0716682b0f7c39ca5c740e19d0e44568342
Author: Nicolas Miller <nicolas.miller@codeplay.com>
Date:   Tue Oct 10 11:42:52 2023 +0100

    Add code owners for the CUDA and HIP adapters

testing update in #11585

commit 0677296778fe29058a4a0dff6d0e30456555c2d1
Merge: b38855e 49770f5
Author: Kenneth Benzie (Benie) <k.benzie@codeplay.com>
Date:   Tue Oct 10 11:38:07 2023 +0100

    Merge pull request #909 from kbenzie/benie/codeowners

    Add CODEOWNERS file

testing update in #11590

commit 6180a78e8bff683c8d944686823b45d3fe939468
Author: Andrey Alekseenko <al42and@gmail.com>
Date:   Mon Oct 2 16:26:36 2023 +0200

    [UR][L0] Fix minor code duplication

    - Second return has no effect
    - Checking the same flag twice has no effect

testing update in #11588

commit 55dccfce3f7371d64cbef7ddc100be39bb5e4c3b
Author: Hugh Delaney <hugh.delaney@codeplay.com>
Date:   Tue Oct 10 10:33:02 2023 +0100

    Re add extended deleters

testing update in #11589

commit 49770f51ac69c485f7ed5c2bc36732e54f8def07
Author: Kenneth Benzie (Benie) <k.benzie@codeplay.com>
Date:   Fri Sep 29 14:21:23 2023 +0100

    Add CODEOWNERS file

    Adapter implementations are being moved into the repo we should mirror
    the code ownership of those implementations. This patch adds a
    CODEOWNERS file which matches the pre adapter move ownership to be used
    as a baseline for future changes for each individual adapter.

this is current baseline

commit b38855ed815ffd076bfde5e5e06170ca4f723dc1
Merge: e6343f4 6a2c548
Author: Piotr Balcer <piotr.balcer@intel.com>
Date:   Thu Oct 5 12:15:42 2023 +0200

    Merge pull request #920 from jsji/localcopy

    [UR][L0] Copy prebuilt L0 to avoid leaking shared folder path

so lets see which PR fails, so at least we know what the culprit is.

kbenzie · 2023-10-19T10:02:11Z

testing update in #11587

commit 4954850f338241d72d506ebcb7162d61afef2924
Merge: 0677296 e5d6a91
Author: Kenneth Benzie (Benie) <k.benzie@codeplay.com>
Date:   Tue Oct 10 11:46:13 2023 +0100

    Merge pull request #906 from 0x12CC/l0_usm_error_checking

    [UR][L0] Propagate errors from `USMAllocationMakeResident`

It does appear to be this change that is causing the regression. I think in order to be able to start making progress getting UR changes into intel/llvm we will need to revert this in UR.

kbenzie · 2023-10-19T12:28:07Z

It does appear to be this change that is causing the regression. I think in order to be able to start making progress getting UR changes into intel/llvm we will need to revert this in UR.

So we tried making the revert and using that in #11344 but the regression persists: https://github.com/intel/llvm/actions/runs/6574062724/job/17858660006?pr=11344

Could we XFAIL this test and investigate it later?

In addition to #11344, there's in now also removing the OpenCL adapter and two more UR PR's we've not merged yet that are blocked by this issue.

jandres742 · 2023-10-20T05:40:07Z

It does appear to be this change that is causing the regression. I think in order to be able to start making progress getting UR changes into intel/llvm we will need to revert this in UR.

So we tried making the revert and using that in #11344 but the regression persists: https://github.com/intel/llvm/actions/runs/6574062724/job/17858660006?pr=11344

Could we XFAIL this test and investigate it later?

In addition to #11344, there's in now also removing the OpenCL adapter and two more UR PR's we've not merged yet that are blocked by this issue.

@kbenzie : if we have only 1 failure with #11344, then yes, we could ask for that to the @intel/llvm-gatekeepers (@smaslov-intel are the @intel/llvm-gatekeepers the correct to determine if we can mark a test as known failure?).

kbenzie · 2023-10-20T11:08:49Z

@kbenzie : if we have only 1 failure with #11344, then yes, we could ask for that to the @intel/llvm-gatekeepers (@smaslov-intel are the @intel/llvm-gatekeepers the correct to determine if we can mark a test as known failure?).

Turns out that #11344 was not using the latest changes from the sycl branch. After pulling in those the remaining regression went away so we won't need to XFAIL the test.

I'll merge the UR revert oneapi-src/unified-runtime#875 shortly.

0x12CC · 2023-10-20T16:33:21Z

Thanks for your help, @kbenzie, @jandres742. I'm not able to reproduce these failures locally so I'll need to spend some time investigating the issue. I'm going to close this PR until the failures have been analyzed and I can propose a solution.

jandres742 · 2023-10-20T20:05:17Z

Thanks for your help, @kbenzie, @jandres742. I'm not able to reproduce these failures locally so I'll need to spend some time investigating the issue. I'm going to close this PR until the failures have been analyzed and I can propose a solution.

thanks @0x12CC . I was not able either on my side. So one theory here would be that we have a data race that is causing this issue, and we might need to try on different types of setups to be able to reproduce it.

0x12CC requested a review from a team as a code owner September 26, 2023 20:14

0x12CC temporarily deployed to WindowsCILock September 26, 2023 20:15 — with GitHub Actions Inactive

0x12CC temporarily deployed to WindowsCILock September 26, 2023 20:43 — with GitHub Actions Inactive

jandres742 reviewed Sep 27, 2023

View reviewed changes

0x12CC mentioned this pull request Sep 28, 2023

[UR][L0] Propagate errors from USMAllocationMakeResident oneapi-src/unified-runtime#906

Merged

0x12CC changed the title ~~[SYCL][L0] Propagate errors from USMAllocationMakeResident~~ [UR][L0] Propagate errors from USMAllocationMakeResident Sep 28, 2023

0x12CC marked this pull request as draft September 28, 2023 20:12

[UR][L0] Propagate errors from USMAllocationMakeResident

fcf7d02

This change ensures that USM allocation APIs don't return `UR_RESULT_SUCCESS` when an error occurs within `USMAllocationMakeResident`. Signed-off-by: Michael Aziz <michael.aziz@intel.com>

0x12CC force-pushed the l0_usm_error_checking branch from 5e661fd to fcf7d02 Compare September 28, 2023 20:13

0x12CC temporarily deployed to WindowsCILock September 28, 2023 20:16 — with GitHub Actions Inactive

0x12CC temporarily deployed to WindowsCILock September 28, 2023 20:44 — with GitHub Actions Inactive

Merge branch 'sycl' into l0_usm_error_checking

ecffbc2

Signed-off-by: Michael Aziz <michael.aziz@intel.com>

0x12CC temporarily deployed to WindowsCILock September 29, 2023 13:52 — with GitHub Actions Inactive

0x12CC temporarily deployed to WindowsCILock September 29, 2023 14:23 — with GitHub Actions Inactive

Merge branch 'sycl' into l0_usm_error_checking

d39924a

Signed-off-by: Michael Aziz <michael.aziz@intel.com>

0x12CC temporarily deployed to WindowsCILock September 29, 2023 15:11 — with GitHub Actions Inactive

0x12CC temporarily deployed to WindowsCILock September 29, 2023 16:16 — with GitHub Actions Inactive

Update commit tag

95e77c1

Signed-off-by: Michael Aziz <michael.aziz@intel.com>

0x12CC marked this pull request as ready for review October 10, 2023 14:45

Merge branch 'sycl' into l0_usm_error_checking

0e801aa

Signed-off-by: Michael Aziz <michael.aziz@intel.com>

0x12CC requested a review from jandres742 October 10, 2023 14:53

0x12CC temporarily deployed to WindowsCILock October 10, 2023 15:00 — with GitHub Actions Inactive

0x12CC temporarily deployed to WindowsCILock October 10, 2023 15:24 — with GitHub Actions Inactive

0x12CC closed this Oct 16, 2023

0x12CC reopened this Oct 16, 2023

0x12CC added 2 commits October 16, 2023 13:31

Merge branch 'sycl' into l0_usm_error_checking

d1a7ec2

Signed-off-by: Michael Aziz <michael.aziz@intel.com>

Update UR version

27c8d98

Signed-off-by: Michael Aziz <michael.aziz@intel.com>

0x12CC temporarily deployed to WindowsCILock October 16, 2023 20:37 — with GitHub Actions Inactive

jandres742 approved these changes Oct 16, 2023

View reviewed changes

0x12CC temporarily deployed to WindowsCILock October 16, 2023 21:16 — with GitHub Actions Inactive

Merge branch 'sycl' into l0_usm_error_checking

aa77f3f

Signed-off-by: Michael Aziz <michael.aziz@intel.com>

0x12CC temporarily deployed to WindowsCILock October 17, 2023 14:39 — with GitHub Actions Inactive

0x12CC temporarily deployed to WindowsCILock October 17, 2023 15:10 — with GitHub Actions Inactive

EwanC mentioned this pull request Oct 17, 2023

[SYCL][Graph][L0] Reset L0 events for UR sync-points #11553

Merged

This was referenced Oct 19, 2023

[UR][HIP] Enable kernel finalization using comgr oneapi-src/unified-runtime#940

Merged

Revert "[UR][L0] Propagate errors from USMAllocationMakeResident" oneapi-src/unified-runtime#972

Merged

0x12CC closed this Oct 20, 2023

0x12CC mentioned this pull request Oct 31, 2023

[UR][L0] Propagate OOM errors from USMAllocationMakeResident oneapi-src/unified-runtime#1022

Merged

0x12CC deleted the l0_usm_error_checking branch November 2, 2023 16:59

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[UR][L0] Propagate errors from `USMAllocationMakeResident` #11312

[UR][L0] Propagate errors from `USMAllocationMakeResident` #11312

0x12CC commented Sep 26, 2023 •

edited

Loading

jandres742 left a comment

0x12CC commented Oct 16, 2023

jandres742 commented Oct 16, 2023

kbenzie commented Oct 17, 2023

0x12CC commented Oct 17, 2023

jandres742 commented Oct 17, 2023

jandres742 commented Oct 19, 2023 •

edited

Loading

kbenzie commented Oct 19, 2023

kbenzie commented Oct 19, 2023 •

edited

Loading

jandres742 commented Oct 20, 2023

kbenzie commented Oct 20, 2023

0x12CC commented Oct 20, 2023

jandres742 commented Oct 20, 2023

[UR][L0] Propagate errors from USMAllocationMakeResident #11312

[UR][L0] Propagate errors from USMAllocationMakeResident #11312

Conversation

0x12CC commented Sep 26, 2023 • edited Loading

jandres742 left a comment

Choose a reason for hiding this comment

0x12CC commented Oct 16, 2023

jandres742 commented Oct 16, 2023

kbenzie commented Oct 17, 2023

0x12CC commented Oct 17, 2023

jandres742 commented Oct 17, 2023

jandres742 commented Oct 19, 2023 • edited Loading

kbenzie commented Oct 19, 2023

kbenzie commented Oct 19, 2023 • edited Loading

jandres742 commented Oct 20, 2023

kbenzie commented Oct 20, 2023

0x12CC commented Oct 20, 2023

jandres742 commented Oct 20, 2023

[UR][L0] Propagate errors from `USMAllocationMakeResident` #11312

[UR][L0] Propagate errors from `USMAllocationMakeResident` #11312

0x12CC commented Sep 26, 2023 •

edited

Loading

jandres742 commented Oct 19, 2023 •

edited

Loading

kbenzie commented Oct 19, 2023 •

edited

Loading