Skip to content

Conversation

@JackAKirk
Copy link
Contributor

@JackAKirk JackAKirk commented Jun 25, 2024

#1429 enabled the queries for p2p, but didn't correspondingly update the enable/disable functions.
This means that the portable P2P workflows, as exampled in the dpc++ tests, e.g. https://github.com/intel/llvm/blob/sycl/sycl/test-e2e/USM/P2P/p2p_access.cpp, are broken on L0 devices.

This fixes that by implementing these functions as a no-op and returning UR_RESULT_SUCCESS. This is the expected behaviour since L0 enables p2p on all devices within a platform by default (see e.g. discussions here intel/llvm#6104).

@JackAKirk JackAKirk requested a review from a team as a code owner June 25, 2024 14:59
@github-actions github-actions bot added the level-zero L0 adapter specific issues label Jun 25, 2024
Copy link
Contributor

@nrspruit nrspruit left a comment

Choose a reason for hiding this comment

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

LGTM, can you link your accompanying intel/llvm PR showing the e2e test results?

@pbalcer
Copy link
Contributor

pbalcer commented Jun 25, 2024

All CI failures are unrelated. The E2E tests that failed are known to be broken in our system (I'll add a suppress).

Failed Tests (9):
  SYCL :: Matrix/SG32/get_coord_int8_matB.cpp
  SYCL :: Matrix/element_wise_all_ops_1d.cpp
  SYCL :: Matrix/element_wise_all_ops_1d_cont.cpp
  SYCL :: Matrix/element_wise_all_ops_scalar.cpp
  SYCL :: Matrix/get_coord_int8_matB.cpp
  SYCL :: Matrix/joint_matrix_apply_bf16.cpp
  SYCL :: Matrix/joint_matrix_apply_two_matrices.cpp
  SYCL :: Matrix/joint_matrix_bfloat16.cpp
  SYCL :: Matrix/joint_matrix_bfloat16_array.cpp

@JackAKirk
Copy link
Contributor Author

LGTM, can you link your accompanying intel/llvm PR showing the e2e test results?

The intel/llvm CI only has a single gpu, therefore these results return early. They have to be tested locally.

@JackAKirk JackAKirk requested a review from a team as a code owner June 25, 2024 16:44
@JackAKirk JackAKirk requested a review from ldrumm June 25, 2024 16:44
@github-actions github-actions bot added the hip HIP adapter specific issues label Jun 25, 2024
@JackAKirk
Copy link
Contributor Author

LGTM, can you link your accompanying intel/llvm PR showing the e2e test results?

The intel/llvm CI only has a single gpu, therefore these results return early. They have to be tested locally.

Apparently UR ci has more devices so the ur test should run, but I realized the macro strings were missing. I also forgot to add it for hip in #1369 so I've added the string for hip in this PR too.

Copy link
Contributor

@nrspruit nrspruit left a comment

Choose a reason for hiding this comment

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

LGTM for level zero

@JackAKirk JackAKirk added the ready to merge Added to PR's which are ready to merge label Jul 2, 2024
@JackAKirk JackAKirk requested a review from ldrumm July 3, 2024 13:46
@kbenzie kbenzie merged commit 06d0c05 into oneapi-src:main Jul 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

hip HIP adapter specific issues level-zero L0 adapter specific issues ready to merge Added to PR's which are ready to merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants